Allow specification of flow exponent for pressure drop element#2144
Allow specification of flow exponent for pressure drop element#2144mwetter wants to merge 6 commits into
Conversation
|
@jelgerjansen : Can you please review this PR. |
|
@mwetter please check with @LoneMeertens or @annadellisola if they can do a KU Leuven review (as I left the university 2 months ago) |
LoneMeertens
left a comment
LoneMeertens
left a comment
There was a problem hiding this comment.
Thank you for this valuable contribution. I have carefully reviewed both the implementation and the associated validation models. Overall, this appears to be a very strong and well-structured addition, supported by thorough validation efforts. In particular, the inclusion of inverse-function consistency and derivative verification is highly appreciated and adds significant confidence in the robustness of the implementation.
I only have a few minor suggestions.
| However, inside the turbulent region, the results differ slightly because | ||
| <a href=\"modelica://IBPSA.Fluid.BaseClasses.FlowModels.powerLaw_dp\"> | ||
| IBPSA.Fluid.BaseClasses.FlowModels.powerLaw_dp</a> | ||
| uses a 5th order polynominal to ensure C2 continuity, while |
There was a problem hiding this comment.
Small typo: change "polynominal" to "polynomial".
| and | ||
| <a href=\"modelica://IBPSA.Fluid.BaseClasses.FlowModels.powerLaw_m_flow\"> | ||
| IBPSA.Fluid.BaseClasses.FlowModels.powerLaw_m_flow</a> | ||
| are not invertible. |
There was a problem hiding this comment.
It may be worth briefly explaining why the functions are not invertible around the origin (i.e., because of the regularization used to ensure smoothness). This would make the documentation a bit more informative for readers unfamiliar with the implementation.
| "Pressure difference between input and output to the functions"; | ||
| Modelica.Units.SI.Time dTime=2; | ||
| parameter Real k = 0.5; | ||
| parameter Real n(min=1, max=2) = 1/0.8 |
There was a problem hiding this comment.
Have the limiting cases n = 1 and n = 2 also been tested? Since these are the bounds of the supported interval, it would be good to know whether they have been verified as well
| Modelica.Units.SI.Pressure deltaDp(displayUnit="Pa") | ||
| "Pressure difference between input and output to the functions"; | ||
| Modelica.Units.SI.Time dTime=2; | ||
| parameter Real k = 0.5; |
There was a problem hiding this comment.
Could you add some spacing for alignment?
| Modelica.Fluid</a> | ||
| can be used and combined with models from the | ||
| <code>IBPSA</code> library. | ||
| <code>Buildings</code> library. |
| InvertingBasicFlowFunction_m_flow | ||
| InvertingPowerLaw_dp | ||
| InvertingPowerLaw_m_flow | ||
| PowerLaw_dp |
There was a problem hiding this comment.
Would it make sense to add a validation test model for PowerLaw_m_flow, analogous to PowerLaw_dp?
| </html>", revisions="<html> | ||
| <ul> | ||
| <li> | ||
| December 19, 2025, by Michael Wetter:<br/> | ||
| First implementation. |
There was a problem hiding this comment.
Could you clarify why the revision here differs from the revisions used in the other models? Just want to check whether this is intentional or if it should be aligned.
| </li> | ||
| </ul> | ||
| </html>"), | ||
| Icon(graphics={ Line( |
| </li> | ||
| </ul> | ||
| </html>"), | ||
| Icon(graphics={ Line( |
| </li> | ||
| </ul> | ||
| </html>"), | ||
| Icon(graphics={ Line( |
3 participants
This closes #2131
See lbl-srg/modelica-buildings#4620 for the changes and rationale.