ModelicaSpecification icon indicating copy to clipboard operation
ModelicaSpecification copied to clipboard

Limiting connections (simpler)

Open HansOlsson opened this issue 3 years ago • 4 comments

This is one of two complementary PRs for mayOnlyConnectOnce which was split off from #3129

As I see it there are two possibilities here:

  • Have a special case for stream-connectors; #3212
  • Remove all the special cases for stream-connectors (this PR)

Having a vote (until end of August); see below.

My previous thoughts about this:

The question is if the rules for the annotation to restrict maximum number of connections makes sense, as potential replacement for the cardinality-assertions in Modelica.Fluid.

Thinking more I'm a bit unsure. On one hand the new rules make sense, and on the other hand it would mean that an example in MSL is misleading. It's a sort of trade-off: how much effort do we want to spend on avoiding potential pit-falls?

The basic idea with the restriction is to replace cardinality-checks.

And since Modelica mostly deal with connection-sets instead of connections the idea was instead of restricting the number of connections to the connector it restricts the number of connectors in its connection set (excluding sensors for fluid, since otherwise it would break more). That implies that the top-two variants in Modelica.Fluid.Examples.Explanatory.MeasuringTemperature would trigger it (if using the annotation instead of the current cardinality-assert). The bottom one would not - since the junction makes the intention clear. The sensor-part means that Modelica.Fluid.Examples.HeatingSystem will not trigger this new annotation.

The MeasuringTemperature example is a bit special: since there is no other dynamics we never get flow directly from one tank to the next - but only from/to the mass-flow.

I can see the following possibilities for the check in OpenTank (well, its base-class):

  • Remove it completely. Users should know what they do.

  • Use new annotation as proposed in #3212 - only one other connector, excluding sensors. Will require junctions in these cases (if we use the new annotation).

  • Check that only connected to one other connector (similar to current cardinality; this simpler PR). That means that you can circumvent the cardinality-check by instead of connecting two connectors to a tank-port you add a temperature sensor and connect all of them (and the tank-port) to the sensor. On one hand using connector-sets seem more correct but:

  • The rules are more complicated.

  • You still cannot directly connect a sensor graphically to a tank-port due to connectorSizing not considering that case; so in practice it doesn't gain much.

HansOlsson avatar Jul 01 '22 12:07 HansOlsson

Poll until end of August, select one (or more) of the emotes as follows:

  • Thumb down: Don't anything for this.
  • Rocket - this PR; simpler rules limiting connections without anything special for streams-connectors.
  • Eyes - #3212 special rules for streams-connectors that makes sense, but might require more work.

HansOlsson avatar Jul 01 '22 12:07 HansOlsson

Wouldn't it be a good idea to also seek input from the MAP-Lib people on this?

henrikt-ma avatar Aug 16 '22 07:08 henrikt-ma

Wouldn't it be a good idea to also seek input from the MAP-Lib people on this?

Yes in particular people working on fluid-models, @casella can you help with getting in touch with them?

Thinking more I would be reluctant to add the proposed rules for streams-connector #3212 unless the members of MAP-Lib agree, and believe that the simple rule without anything special for streams-connector should do for now.

HansOlsson avatar Aug 17 '22 13:08 HansOlsson

Just to clarify (from #3129 ) - there are two differences for this rule compared to cardinality-tests:

  1. It is based on connection sets, not connection, so the top two examples in Modelica.Fluid.Examples.Explanatory.MeasuringTemperature will trigger. (The sensors don't matter for this.)
  2. Sensors are excluded so Modelica.Fluid.Examples.HeatingSystem will not trigger for tank.ports[1] due to sensor_T_return.

That makes it more impactful - and also leads to the rather obvious question - how would you model the top-example in Modelica.Fluid.Examples.Explanatory.MeasuringTemperature with this restriction? As far as I can see a hack solution is to have two TeeJunctionIdeal - one before each tank - to circumvent the check. A non-hack solution would require a JunctionIdeal with just two ports; clearly that is something that needs to be considered in the library (when adding this annotation).

Note that if we had use connection instead of connection sets I don't see that we need to do anything special for streams-connectors. Basically users just connect twice to the sensor or something else on the other side as is currently done in these two examples.

HansOlsson avatar Aug 19 '22 12:08 HansOlsson

We selected the other option.

HansOlsson avatar Sep 29 '22 19:09 HansOlsson