Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility to Open Modelica #10

Closed
dzimmer opened this issue May 12, 2021 · 13 comments
Closed

Compatibility to Open Modelica #10

dzimmer opened this issue May 12, 2021 · 13 comments
Assignees
Labels
enhancement New feature or request

Comments

@dzimmer
Copy link
Contributor

dzimmer commented May 12, 2021

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.

@dzimmer dzimmer added the enhancement New feature or request label May 12, 2021
@dzimmer dzimmer self-assigned this May 12, 2021
@mimeissner
Copy link
Collaborator

mimeissner commented Jun 2, 2021

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->Tests
  • Interface
  • Boundaries except Volume & PhaseSeperator
  • Topology except DynamicTopology & TestJunctionNM
  • Processes except ConductionElement & TransportDelay & Nozzle
  • Heat exchangers (when replaceable support is off)
  • FlowControl
  • Sensor does not work
  • undirected-> Interfaces
  • undirected-> Boundaries except Volume & PhaseSeperator
  • undirected-> Topology
  • undirected-> Processes except conductionElement & transportDeley
  • undirected-> Heat exchangers(when replaceable support is off)
  • undirected-> FlowControl
  • undirected-> Sensors does not work

Examples:

  • SimpleStream: does not run
  • SimpleAirCycle: does not run
  • SimpleEngine: runs
  • SimpleGasTurbine: does not run
  • SimpleCoolingCycle: runs
  • WaterHammer: runs
  • EspressoMachine: runs
  • VenturiPump: runs
  • VaporCycle: does not run
  • HeatPump: does not run
  • ReverseHeatpump:does not run

why models dont run:

  • TestVolumes: Volume density_derp_h not conditionally removed
  • TestPhaseSeperator: Initialization problem -> somehow related with h=0
  • TestDynamicTopology & TestJunctionNM: MoistAir bug
  • ConductionElement: der(p) not conditionally removed
  • TransportDelay: undefined reference to spatialDistribution
  • Nozzle: MoistAir bug
  • CrossFlowNTU_zeroMassFlow: initializaion problem (unknown cause)
  • Condenser, Evaporator: MoistAir bug when replaceable support disabled
  • TestSensors: progress: error code 2 (with workaround) (unknown cause)
  • Bidi-> Volumes: density_derp_h not conditional removed (same as directed)
  • Bidi-> VolumesDirectCoupeling: sim super slow
  • Bidi-> PhaseSeperator: init problem -> somehow related with h=0 (same as directed)
  • BIDI-> ConductionElement: der(p) not conditionally removed (same as directed)
  • BIDI-> TransportDelay: undefined reference to spatialDistribution (same as directed)
  • BIDI-> Condenser, Evaporator: MoistAirbug when replaceable support disabled (same as directed)
  • BIDI-> Sensors: progress: error code 2 (with workaround) (unknown cause, same as directed)
  • SimpleStream: MoistAir bug
  • SimpleAirCycle: MoistAir bug
  • SimpleGasTurbine: Volume model density_derp_h not conditionally removed
  • VaporCycle: runs forever
  • HeatPump: runs forever
  • ReverseHeatpump: initialization problem (unknown cause)

known issues:

  • 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
  • Sensors use a function to determine their RealOutput units. OM only supports string literals
  • We still have a lot of conflicting start/nominal values
  • setting the assertion level from a parameter in dropOfCommons does not seem to work (setting it directly in the assert function call works)
  • MoistAir bug: some problem with state record->X in equations. we have only been able to reproduce with MoistAir, so the bug might also be there. minimal example in Topology -> Test-> test (see documentation of example); could also be a error with nested records or arrays in records
  • density_derp_h not conditionally removed / der(p) not conditionally removed: handling of conditionally removed equations is different than in Dymolya: it seems that the equation is removed after it is checked for realizability, instead before
  • max has a problem with types: e.g. check on function Processes->Internal->TurboComponent->dp_tau_nominal_flow
  • 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 replacad already it should not have to be replacad again by user (e.g. pump, fan, compressor)
  • spatialDistribution is undefined
  • missing feature enable annotation; final hides parameters and not displaying removed connectors
  • OM and Dymola mirror icons over a different axis
  • h=0 bug: when h is set by a table, simulation stops with warning h=0, even if h is never 0 in table: possibly related with start values. minimal example in Boundaries-> Test-> test

@sjoelund
Copy link

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
Copy link
Collaborator

mimeissner commented Jul 5, 2021

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
  • 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
  • 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

@sjoelund
Copy link

sjoelund commented Jul 6, 2021

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
Copy link

adeas31 commented Jul 6, 2021

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
Copy link
Collaborator

mimeissner commented Jul 6, 2021

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

@mimeissner
Copy link
Collaborator

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
Copy link

adeas31 commented Jul 6, 2021

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.

@mimeissner mimeissner self-assigned this Jan 19, 2022
@casella
Copy link

casella commented Jan 22, 2022

@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
Copy link
Collaborator

@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
Copy link
Collaborator

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

@casella
Copy link

casella commented Feb 22, 2024

@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
Copy link
Contributor Author

dzimmer commented Feb 26, 2024

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.

@dzimmer dzimmer closed this as completed Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants