Source sink flow asserts #132
Conversation
This adds to the Source and Sink components an assert on m_flow direction similar to what is already in the PartialVolume components. Some models in Examples such as SimpleStream and WaterHammer do not have volumes so without this commit there is no check for invalid mass flow direction if users modify the models. With this commit, some topologies may still have invalid flow direction without an assert. To make a complete check, the SISOFlow or Inlet and Outlet connectors need an assert, but that will lead to many more assert messages. This commit is a compromise to catch most errors with fewer messages.
The new assertion for reversed mass flow in Boundaries.Source catches an excursion larger than the default threshold in the ExpressoMachine example. Making the threshold 3 times larger avoids the assertion.
github-actions
bot
added
the
p::Boundaries
ThermofluidStream/Boundaries/Sink.mo
Outdated
| @@ -13,6 +13,8 @@ the outlet the sink is connected to. | |||
| annotation(Dialog(enable = not pressureFromInput)); | |||
| parameter Utilities.Units.Inertance L=dropOfCommons.L "Inertance of pressure" | |||
| annotation (Dialog(tab="Advanced")); | |||
| parameter SI.MassFlowRate m_flow_assert(max=0) = -dropOfCommons.m_flow_reg "Assertion threshold for negative massflows" | |||
There was a problem hiding this comment.
@nieweber The existing PartialVolume has this:
parameter SI.MassFlowRate m_flow_assert(max=0) = -dropOfCommons.m_flow_reg "Assertion threshold for negative massflows"Per your suggestions, Source and Sink will be inconsistent. If we instead change PartialVolume, then it would not be backward compatible.
Please confirm that you want it to be inconsistent or let me know what you would like instead and I'll make the change per your suggestions.
There was a problem hiding this comment.
Oh yes thank you, you are completely right. Sorry, I didn't cross-check with the assert in the PartialVolume. The components should indeed be consistent so we will discuss, if we want to include it in the next minor release (1.1.0) and keep the negative sign or if we shift this change to the next major release and adjust the PartialVolume assert.
I will let you know what we would like to do.
There was a problem hiding this comment.
So I think the best way is to add the assert to the source and sink models as it is implemented in the PartialVolume. We might change the sign of the m_flow_assert parameter in the next major release. I just did some small corrections on the spelling that I will also introduce in the assert in the PartialVolume component.
Thanks a lot for your contribution!
ThermofluidStream/Boundaries/Sink.mo
Outdated
| @@ -13,6 +13,8 @@ the outlet the sink is connected to. | |||
| annotation(Dialog(enable = not pressureFromInput)); | |||
| parameter Utilities.Units.Inertance L=dropOfCommons.L "Inertance of pressure" | |||
| annotation (Dialog(tab="Advanced")); | |||
| parameter SI.MassFlowRate m_flow_assert(max=0) = -dropOfCommons.m_flow_reg "Assertion threshold for negative massflows" | |||
There was a problem hiding this comment.
@nieweber The existing PartialVolume has this:
parameter SI.MassFlowRate m_flow_assert(max=0) = -dropOfCommons.m_flow_reg "Assertion threshold for negative massflows"Per your suggestions, Source and Sink will be inconsistent. If we instead change PartialVolume, then it would not be backward compatible.
Please confirm that you want it to be inconsistent or let me know what you would like instead and I'll make the change per your suggestions.
Co-authored-by: tobolar <[email protected]>
Co-authored-by: tobolar <[email protected]>
This adds to the Source and Sink components an assert on m_flow direction
similar to what is already in the PartialVolume components.
Some models in Examples such as SimpleStream and WaterHammer do
not have volumes so without this commit there is no check for invalid
mass flow direction when users modify the models. This also catches a
mass flow reversal in the EspressoMachine example that is larger than
the default threshold, so the threshold for that example has been
increased so there is no assert.
With this commit, some topologies may still have invalid flow direction
without an assert. To make a complete check, the SISOFlow or Inlet
and Outlet connectors need an assert, but that will lead to many more
assert messages. This commit is a compromise to catch most errors
with fewer messages.