Compatibility to Modelon Impact (Set 1)

[hubertus65]

I imported the Library into Modelon Impact, and had to fix a few minor Modelica issues to make it work.

1. In dropOfCommons, the component AssertionLevel needs a prefix parameter to be legal Modelica. That fixes already a few examples.
2. In Unidirectional.JunctionMN, there are a few uses of sum(V x V), where V is a 1-D vector. V x V is a Real scalar in Modelica, and if I interpret the formulas correctly, the sum can simply be removed to be correct (and Modelon Impact complains about wrong type).
3. Line 258 in Examples.Heatpump has a scalar Real as an argument to max, also not correct for Modelon Impact. And: what do you mean with it?
4. Something about the way you do the display of values on the canvas is not supported by Modelon Impact, even though we have similar components and support DynamicSelect.
5. The workaround in Boundaries.Volume does not work in Modelon Impact, since it always checks both branches at least for a legal lookup. Better to update that in Modelica.Media, or add it in your own local Media package
6. The function dp_tau_centrifugal in package Processes.Internal.TurboComponent computes constants from inputs, which I'd say is illegal in functions but accepted by Dymola. It works fine without the constant modifier.

I can check some more, and might eventually add a pull request. Nice Library!

[dzimmer]

Thanks a lot for your hints. We want to improve tool compatibilty.
We'll work that stuff in but due to vacation season, it may take a bit of time. Feel free to make a pull request.

[hubertus65]

I plan to do some more testing/digging and then do a coordinated one. If I find time to play, ...
BTW: testing on multiple tools tends to generally improve the quality. Our experience at least, with having a minimum of 2 tools per library

[hubertus65]

Ok, I have solutions for items 1, 2, and 4 above, which work in both Dymola and Modelon Impact (by extension also in Amesim and ANSYS Twinbuilder), but need to update the solution for sensor displays to many more places. For item 3: I cannot fix it well if you don't tell me what you mean. I can probably even fix item 5 easily for ideal gases, but that fix is "per medium", and I don't see others that need fixing. With all the above fixes, I think that everything works. The only missing item for me to follow through on all of this is time.

[hubertus65]

I don't know how to link this with my pull request. In any case, the pull request makes this 100% runnable in Modelon Impact.

[mimeissner]

Thank you for your work.
Regarding Item 3: The max seems to be there for historic reasons, I will improve this line.

You can link to the pull request by typing # and the coressponding number, for this case 20: #20 .

[mimeissner]

I checked out bf8f317

For me ThermofluidStream.Boundaries.Tests.Volumes and ThermofluidStream.Undirected.Boundaries.Tests.TestVolumes fail to translate, since SimpleAir does not have the function density_derp_h.
ThermofluidStream.Boundaries.Tests.VolumesDirectCoupling fails to translate, since it tries to modify density_derp_h_from_media, which was removed in the commit.
Do these translate for you?

I think we have to discuss how we proceed for Item 3. Since the solution was only partial a workaround and we want the option to manually set a fixed value for density_derp_h regardless of wheather the workaround is nessesary.

All other Changes will be merged in the master soon.

[mimeissner]

@hubertus65, I took your changes and applied some adaptions.

The version I would merge into main is in the Branch https://github.com/DLR-SR/ThermofluidStream/tree/ModelonImpact. Could you check if this version with my changes is still compatible with Modelon Impact before I merge it into main?

Thank you

[hubertus65]

Regarding the density_derp_h-issue: I think the right way to fix this is to always add a function to the medium, or make sure that all medium implementations have that function, even if it returns a constant. That this is not the case consistently in Modelica.Media is an issue, but it should be fixed there instead. The "density_derp_h_from_media" is an ugly workaround, no matter what. Ideally, the situation should be:

• all medium models define one coherent interface, i.e. the derivative functions needed for changing states are always there. That is what is promised in the interface.
• the interim solution to fix this in Thermofluitstream should be to locally extend from Modelica.Media and declare or redeclare the missing function
• density_derp_h_from_media is then not needed.
  Do you agree? I'll check the Branch later.

[mimeissner]

We fully agree with your solution, as well as the interim solution. Interface defenitions should be complied by all child classes.

The current implementation sees density_derp_h_from_media not as a workaround for the missing functions. Rather we want to be able to set and use a bound for the gradient, instead of using the gradient itself in some cases.

[hubertus65]

Now I see the reason. Such that you can qualify a group of fluids/gases to be suitable. Then it's ok, but I would make the parameter naming more clear/explicit. Or create an artificial medium, that has these "worst case" derivatives/properties encoded. I assume, that some other properties, e.g. viscosity or heat capacity, would be much more important in terms of evaluating performance limits?

[mimeissner]

This parameter is only used for a artificial damping term of the change of mass of a volume. In case two boundaries are directly coupled, it damps very fast, otherwise undaped oscilations and should otherwise influence the results of the model only minor. Therefore most of the time only the magnitude of the derivative is important.
I will make the naming more clear.

[hubertus65]

@hubertus65, I took your changes and applied some adaptions.

The version I would merge into main is in the Branch https://github.com/DLR-SR/ThermofluidStream/tree/ModelonImpact. Could you check if this version with my changes is still compatible with Modelon Impact before I merge it into main?

Thank you

I checked all Examples, and they are all working fine, and produce sensible and reasonable results I have not done an in-depth trajectory comparison as we do for our library products, but

1. All models compile without error
2. All models simulate, and give reasonable results and
3. All on-Canvas displays work and give nice visual feedback.

I'd consider it fully working as-is on Modelon Impact. I think it also still is fully working in Dymola. Fine to merge, and close the associated issue. I'm happy that I was able to make it run completely on our side with very little total time effort. I suggest that you merge your branch and close this issue, and I delete my pull request now.