DLR-SR / ThermofluidStream

The DLR Thermofluid Stream Library
BSD 3-Clause "New" or "Revised" License
65 stars 27 forks source link

Compatibility to Open Modelica #10

Closed dzimmer closed 8 months ago

dzimmer commented 3 years ago

Currently the library works fine with Dymola. However we want to make it available to OpenModelica as well. In this issue we track the corresponding state and progress.

mimeissner commented 3 years ago

progress report with bugfixes and workarounds on branch OM_adaptions and OM version v1.17.0:

which test cases do at least compile (most will then simulate, some still simulate forever or fail initializing):

Tests:

Examples:

why models dont run:

known issues:

sjoelund commented 3 years ago

The results with OM master branch (nightly builds) fix a lot of these issues, in particular the MoistAir bug: https://libraries.openmodelica.org/branches/master/ThermofluidStream_OM_adaptions/ThermofluidStream_OM_adaptions.html

mimeissner commented 3 years ago

I have installed OM version v1.19.0-dev-64bit (02.07.2021): As described most of the issues seem to be fixed. Only some minor issues remain.

The Reason why most simulation fail, is that the correct assertion level is not used in the components, if it is set in the dropOfCommons.

remaining known issues:

Reasons why models still not compile:

sjoelund commented 3 years ago

I have installed OM version v1.19.0-dev-64bit (02.07.2021): As described most of the issues seem to be fixed. Only some minor issues remain.

The Reason why most simulation fail, is that the correct assertion level is not used in the components, if it is set in the dropOfCommons.

remaining known issues:

  • OM and Dymola mirror icons over a different axis
  • missing features: annotation enable, final should hide parameters, removed connectors should be hidden in Diagramm

@adeas31 any comment? I think the axis thing can be resolved by going from left to right when drawing the icon to get it consistent? Been a while since I looked at OMEdit GUI issues.

  • workaround media of dp_tau: different behavior for functions with replaceable packages then Dymola: if the function is constraied by a function where the package is replaced already it should not have to be replaced again by user (e.g. pump, fan, compressor)
  • setting the assertion level from a parameter in dropOfCommons does not seem to work (setting it directly in the assert function call works)
  • Sensors use a function to determine their RealOutput units. OM only supports string literals

@perost should these be opened as bugs for the frontend? Especially 2 and 3 I think should be possible to resolve (2 only when possible to evaluate in the frontend I suppose; 3 should be possible to evaluate the level at runtime as far as I can tell).

  • replaceable support: not enabled by default; without the replaceable media definitions cannot edited on mask and get lost on copying a component; with it enabled all models containing a discretized Hex crash the frontend
  • There seems to be a compatibility Issue with the XRGMedia.

Reasons why models still not compile:

  • HeatPump: compatibility issue within XRGMedia
  • TestXRGMedia: compatibility issue within XRGMedia
adeas31 commented 3 years ago

OM and Dymola mirror icons over a different axis

Which icons are mirrored?

missing features: annotation enable, final should hide parameters, removed connectors should be hidden in Diagram

Yes. This is the known conditional connectors issue.

mimeissner commented 3 years ago

examples are the upper UnidirectionalSensorAdapter in the model ThermofluidStream.Undirected.Sensors.Tests.TestSensor and the crancDrive in ThermofluidStream.Examples.SimpleEngine

mimeissner commented 3 years ago

concerning the point "Sensors use a function to determine their RealOutput units. OM only supports string literals"

the warning is no longer apperent in the nightly build of 02.07. I have installed now, but the unit does not show up in the plotting window.

adeas31 commented 3 years ago

examples are the upper UnidirectionalSensorAdapter in the model ThermofluidStream.Undirected.Sensors.Tests.TestSensor and the crancDrive in ThermofluidStream.Examples.SimpleEngine

That is a bug in OM. See https://trac.openmodelica.org/OpenModelica/ticket/5816 I will try to fix it asap.

casella commented 2 years ago

@mimeissner, I see that now the main and OM-adaptions branches are fully aligned, and produce the same results in our CI reporting: https://libraries.openmodelica.org/branches/master/ThermofluidStream/ThermofluidStream.html vs. https://libraries.openmodelica.org/branches/master/ThermofluidStream_OM_adaptions/ThermofluidStream_OM_adaptions.html

I can't see any reason why you should have OMC-specific adaptations; in fact, we should all strive or a situations where libraries can run on multiple Modelica tools without any kind of adaptation. So, I would suggest that we stop testing the OM-adaption branch on our servers, and that you remove it from the repo.

What do you think?

mimeissner commented 2 years ago

@casella, if it is not to harmful to your process (e.g. runtime of the test) I would prefer to keep the OM_adaptions open for some time, so I can use it to test some ideas before merging them into main (where we dont want to rewrite history). The naming of the branch would be miss-leading, but I think this would be usefull to keep our history of the main branch clean for now.

We also aim for our library to run in all tools without tool-spcific adaptions, so longterm the two branchen will not diverge.

mimeissner commented 2 years ago

I just closed the branch OM_adaptions since we dont use it anylonger. @sjoelund can you remove it from the OpenModelica Library testing?

casella commented 8 months ago

@mimeissner, @dzimmer, this is the current status of ThermoFluidStream with the latest OMC nightly build

immagine

Seems to me quite good. Can we close this ticket for good?

dzimmer commented 8 months ago

I have downloaded and tested the latest official release of Open Modelica v1.22.2 and was pleasently surprised. Now the support for all the replaceable stuff is good. There is support for the sub-dialogs although it seems to be read-only at the moment. Also compilation speed is fine.

I think we can justify closing this issue and if there are further points to be taken care of they can be addressed by a separate issue.