Skip to content

Conversation

@beutlich
Copy link
Member

@beutlich beutlich commented Aug 18, 2019

Fix #359.

@christiankral Please double-check Modelica.Electrical.Machines.BasicMachines.Components.DamperCage which was the only non-trivial conversion since it has both lossPower and LossPower declared.

Would also be good if one of the reviewers could run "Check model" in Dymola.

@beutlich beutlich added L: Electrical.Analog Issue addresses Modelica.Electrical.Analog L: Electrical.Machines Issue addresses Modelica.Electrical.Machines L: Electrical.PowerConverters Issue addresses Modelica.Electrical.PowerConverters L: Magnetic.FluxTubes Issue addresses Modelica.Magnetic.FluxTubes task General work that is not related to a bug or feature L: Electrical.QuasiStatic Issue addresses Modelica.Electrical.QuasiStatic labels Aug 18, 2019
@beutlich beutlich added this to the MSL4.0.0 milestone Aug 18, 2019
@beutlich beutlich self-assigned this Aug 18, 2019
@beutlich beutlich changed the title Convert Modelica.Electrical.Analog.Interfaces.ConditionalHeatPort Remove Interfaces.ConditionalHeatPort from Modelica.Electrical.Analog and Modelica.Magnetic.FluxTubes Aug 18, 2019
@beutlich beutlich force-pushed the convert-ConditionalHeatPort branch from b5254db to 3e7a414 Compare August 18, 2019 14:16
@beutlich beutlich added specification Issue (also) addresses the Modelica language specification tool-issue Issue targets a specific Modelica tool only labels Aug 18, 2019
@beutlich
Copy link
Member Author

This is getting worse:

  • Unfortunately the connector position of the heatport is different in Modelica.Electrical.Analog.Interface.ConditionalHeatPort and Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPort.
  • Unfortunately we cannot move the connector position after inheritance (see Modification of connector location after inheritance ModelicaSpecification#377).
  • Unfortunately, when using the IconMap/DiagramMap annotation of the extends clause, I get three different results in Dymola, SimulationX and Wolfram SystemModeler.
  1. Dymola requires
extends Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPort
  annotation(IconMap(extent={{0,-100}, {200, 100}}), DiagramMap(extent={{0,-100}, {200, 100}}));

to display the heatport at the correct position for backward compatibility with Modelica.Electrical.Analog.Interface.ConditionalHeatPort. However also the connectors and connection lines from other base classes are moved now. Probably a tool issue @HansOlsson ?

  1. SimulationX requires
extends Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPort
  annotation(IconMap(extent={{-200,-100}, {0, 100}}), DiagramMap(extent={{-200,-100}, {0, 100}}));

which is the opposite direction compared to Dymola. Is this a tool or specification issue @ooer?

  1. Wolfram SystemModeler does not move the inherited heatport connector at all.

@beutlich
Copy link
Member Author

beutlich commented Aug 18, 2019

I took the time and crreated a test model comparing the four different implementations using a simple Resistor as an example. Compare the screen shots from Dymola and SimulationX:

  1. Resistor1 is the original implementation of MSL v3.2.3 and is the reference.
    grafik
    grafik
  2. Resistor2 no longer extends from Modelica.Electrical.Analog.Interfaces.ConditionalHeatPort but from Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPort.
    grafik
    grafik
  3. Resistor 3 also sets the IconMap/DiagramMap annotation such that the heatPort is moved to the reference position (in Dymola). However, in Dymola also the blue electrical connection lines are modified as well.
    grafik
    grafik
  4. Resistor 4 also sets the IconMap/DiagramMap annotation such that the heatPort is moved to the reference position (in SimulationX). However, in SimulationX the red thermal connection line is drawn as for Resistor2, i.e., ignoring the IconMap annotation.
    grafik
    grafik

@HansOlsson @ooer Can you please confirm the tool issue or specification issue. Thank you.

@christiankral
Copy link
Contributor

christiankral commented Aug 26, 2019

@beutlich I wonder if it is save to remove Modelica.Electrical.Analog.Interfaces.PartialConditionalHeatPort from the MSL which is still in. There is just Modelica.Electrical.Analog.Batteries.BatteryOCV_SOCtable relying on it. I guess it is no problem to keep the broken Analog.Batteries package, as it will be replaced by #3054 Electrical.Batteries.

@christiankral
Copy link
Contributor

@beutlich shall we remove the keyword Partial from the following models?

  • Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPort
  • Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPortWithoutT
  • Modelica.Thermal.HeatTransfer.Interfaces.PartialConditionalHeatPort

@christiankral
Copy link
Contributor

@beutlich Even though the partial model Modelica.Thermal.HeatTransfer.Interfaces.PartialConditionalHeatPort is only used in polyphase models by now, but it may be used in any application which uses a vector of heat ports. It think it is therefore better to move this partial model to Modelica.Thermal.HeatTransfer.Interfaces as well. A possible naming of the vector heat port model (and the derivatives Elementary... and ...WithoutT for full compatibility) is:

  • ElementaryConditionalVectorHeatPort
  • ElementaryConditionalVectorHeatPortWithoutT
  • ConditionalVectorHeatPort

@beutlich
Copy link
Member Author

beutlich commented Aug 26, 2019

@beutlich shall we remove the keyword Partial from the following models?

* `Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPort`

* `Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPortWithoutT`

* `Modelica.Thermal.HeatTransfer.Interfaces.PartialConditionalHeatPort`

OK from my side, but in a separate PR please.

@beutlich
Copy link
Member Author

@beutlich Even though the partial model Modelica.Thermal.HeatTransfer.Interfaces.PartialConditionalHeatPort is only used in polyphase models by now, but it may be used in any application which uses a vector of heat ports. It think it is therefore better to move this partial model to Modelica.Thermal.HeatTransfer.Interfaces as well. A possible naming of the vector heat port model (and the derivatives Elementary... and ...WithoutT for full compatibility) is:

* `ElementaryConditionalVectorHeatPort`

* `ElementaryConditionalVectorHeatPortWithoutT`

* `ConditionalVectorHeatPort`

OK from my side, but in a separate PR please. What about ElementaryConditionalHeatPortV?

@beutlich
Copy link
Member Author

@beutlich I wonder if it is save to remove Modelica.Electrical.Analog.Interfaces.PartialConditionalHeatPort from the MSL which is still in. There is just Modelica.Electrical.Analog.Batteries.BatteryOCV_SOCtable relying on it. I guess it is no problem to keep the broken Analog.Batteries package, as it will be replaced by #3054 Electrical.Batteries.

If so, we should move it to ObsoleteModelica4.mo like I did with the other replaced classes.

@beutlich
Copy link
Member Author

This is getting worse:

* Unfortunately the connector position of the heatport is different in Modelica.Electrical.Analog.Interface.ConditionalHeatPort and Modelica.Thermal.HeatTransfer.Interfaces.PartialElementaryConditionalHeatPort.

* Unfortunately we cannot move the connector position after inheritance (see [modelica/ModelicaSpecification#377](https://github.com/modelica/ModelicaSpecification/issues/377)).

* Unfortunately, when using the IconMap/DiagramMap annotation of the [extends clause](https://specification.modelica.org/master/Ch18.html#extends-clause), I get three different results in Dymola, SimulationX and Wolfram SystemModeler.

@christiankral In case you missed my #3097 (comment), I doubt we can merge this PR without a break in backward compatibility of the heatport position.

@beutlich
Copy link
Member Author

Closing, since impossible to fix for MSL v4.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L: Electrical.Analog Issue addresses Modelica.Electrical.Analog L: Electrical.Machines Issue addresses Modelica.Electrical.Machines L: Electrical.PowerConverters Issue addresses Modelica.Electrical.PowerConverters L: Electrical.QuasiStatic Issue addresses Modelica.Electrical.QuasiStatic L: Magnetic.FluxTubes Issue addresses Modelica.Magnetic.FluxTubes specification Issue (also) addresses the Modelica language specification task General work that is not related to a bug or feature tool-issue Issue targets a specific Modelica tool only wontfix Issue that will not be fixed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move conditional heat port models to Modelica.Thermal.HeatTransfer.Interfaces

2 participants