Added pipe standard type 'heating pipe' #569
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #569 +/- ##
==========================================
Coverage ? 85.10%
==========================================
Files ? 90
Lines ? 6209
Branches ? 0
==========================================
Hits ? 5284
Misses ? 925
Partials ? 0 ☔ View full report in Codecov by Sentry. |
|
Thanks for the contribution. I think this is a good idea. In my opinion, however, I think we should not add a second pipe.csv. I would rather extend the exisiting pipe.csv and add Nones for the pipes which do not have U values. The reason is that, if you implement it your way, at two different places the user needs to decide whether she/he has a heating or a non-heating system - at the beginning, when he/she creates the net, and during the pipeflow (this could lead to unitended errors). Furthermore, in future it could be that it might be interesting to investigate a heating/gas grid. For example in case of a steam network. In that case we would need to change your implementation again, which would affect convert_format and make thinks quite comlicated. This is my feeling. @dlohmeier, what do you think? |
-using toolbox for alpha calculation results in circular import(how to fix?) -raise userwarning stop the process?
-fixed circular import (moved import into function) -fixed userwarning -changed u.type from string to float
…to parameter_u
-fixed circular import (moved import into function) -fixed userwarning -changed u.type from string to float
-dropped elif case
|
@SimonRubenDrauz @dlohmeier I updated the pull request. Could you take a look again? |
pandapipes/create.py
Outdated
| add_new_component(net, Pipe) | ||
|
|
||
| index = _get_index_with_check(net, "pipe", index) | ||
| _check_branch(net, "Pipe", index, from_junction, to_junction) | ||
| _check_std_type(net, std_type, "pipe", "create_pipe") | ||
|
|
||
| pipe_parameter = load_std_type(net, std_type, "pipe") | ||
|
|
||
| if pd.notna(pipe_parameter['u_w/mk']) and alpha_w_per_m2k == 0.0: |
There was a problem hiding this comment.
I would prefer to use u_w_per_mk here in analogy to the naming of other parameters.
There was a problem hiding this comment.
when renaming u to alpha in table, this is not needed anymore
pandapipes/std_types/std_types.py
Outdated
| @@ -5,7 +5,7 @@ | |||
| import os | |||
| import warnings | |||
| import re | |||
|
|
|||
| import numpy as np | |||
There was a problem hiding this comment.
I think this import statement is not needed, would look better wrt codacy to remove.
pandapipes/toolbox.py
Outdated
| @@ -615,3 +615,6 @@ def get_internal_tables_pandas(net, convert_types=True): | |||
| tbl[col] = tbl[col].astype(np.bool_) | |||
|
|
|||
| return node_table, branch_table | |||
|
|
|||
| def calculate_alpha(d, u): | |||
There was a problem hiding this comment.
I would rename this function and signature: def alpha_from_u(u_w_per_mk, s_m) . As far as I can recall using s for the wall thickness is more common than d, which is usually the diameter, and the unit would be very helpful here so that the user doesn't have to check the code. Otherwise, it could make sense to name it precisely as thickness.
There was a problem hiding this comment.
And to avoid confusion, I would write some docs, describing the meaning of alpha, u and s.
| 1600_ST<16;1600;1620.0;1588.0;101.25;ST | ||
| 1800_ST<16;1800;1820.0;1785.0;104.0;ST | ||
| 2000_ST<16;2000;2020.0;1980.0;101.0;ST | ||
| std_type;nominal_width_mm;outer_diameter_mm;inner_diameter_mm;standard_dimension_ratio;material;u_w/mk |
There was a problem hiding this comment.
u_w/mk to alpha_w_per_m2k
Added a standard type for heating pipes.
The reason for this addition is, that in literature and industry the parameter for heat loss specified is predominantly U [W/mK] instead of alpha [W/m²K].