Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Conversation

dlohmeier
Copy link
Collaborator

No description provided.

EPrade
EPrade previously approved these changes Jul 30, 2024
@SimonRubenDrauz
Copy link
Collaborator

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
Copy link
Collaborator Author

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

Copy link

codecov bot commented Aug 20, 2024

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 ⚠️
Additional details and impacted files
@@             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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dlohmeier
Copy link
Collaborator Author

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

…(not density at normal conditions); added tests for correct behavior in temperature calculation, including convergence to good fluid properties from bad starting values in bidirectional mode
dlohmeier and others added 5 commits August 21, 2024 12:22
…ame value instead of calculating density for same value many times based on list
…l_density

# Conflicts:
#	CHANGELOG.rst
#	src/pandapipes/component_models/flow_control_component.py
#	src/pandapipes/component_models/heat_consumer_component.py
#	src/pandapipes/component_models/heat_exchanger_component.py
#	src/pandapipes/component_models/pipe_component.py
#	src/pandapipes/component_models/pressure_control_component.py
#	src/pandapipes/component_models/pump_component.py
#	src/pandapipes/component_models/valve_component.py
#	src/pandapipes/pf/result_extraction.py
@SimonRubenDrauz SimonRubenDrauz merged commit 1c40dbc into e2nIEE:develop Sep 6, 2024
29 checks passed
@SimonRubenDrauz SimonRubenDrauz deleted the hotfix/vdot_extraction_imcompressible_real_density branch September 6, 2024 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants