search

e2nIEE / pandapipes

A pipeflow calculation tool that complements pandapower in the simulation of multi energy grids
https://www.pandapipes.org
Other
139 stars 60 forks source link

Bugfix: result extraction for incompressible media now based on real density #640

Closed dlohmeier closed 1 day ago

SimonRubenDrauz commented 1 month ago

I see your point but I don't agree totally with your adaptions. vf is actually defined as standard flow rate. This is also extracted as result in liquid fluids. If you change vf the way you do, the results are misleading in my opinion. I guess we have to possibilities: 1.) Either we use vf differently. In liquids it is considered as the volumetric flow rate (and displayed accordingly), in gases it is the standard flow rate 2.) We consider both in the results and display both.

In terms of the velocities I agree and we should correct the bug accordingly

dlohmeier commented 2 weeks ago

pandapower only supports python >=3.9 on develop. The tutorial tests are still running with pandapower develop, which is why also required tests fail, not just the relying tests. I guess we should change this to pandapower master? Should I do it in this PR? @SimonRubenDrauz @EPrade

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 85.63%. Comparing base (12c8256) to head (4a1cf98).

Files Patch % Lines
...pes/component_models/pressure_control_component.py 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #640 +/- ## =========================================== - Coverage 86.02% 85.63% -0.40% =========================================== Files 90 90 Lines 6497 6500 +3 =========================================== - Hits 5589 5566 -23 - Misses 908 934 +26 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dlohmeier commented 2 weeks ago

I see your point but I don't agree totally with your adaptions. vf is actually defined as standard flow rate. This is also extracted as result in liquid fluids. If you change vf the way you do, the results are misleading in my opinion. I guess we have to possibilities: 1.) Either we use vf differently. In liquids it is considered as the volumetric flow rate (and displayed accordingly), in gases it is the standard flow rate 2.) We consider both in the results and display both.

In terms of the velocities I agree and we should correct the bug accordingly

This is now handled differently for compressible / incompressible media