pandapipes icon indicating copy to clipboard operation
pandapipes copied to clipboard

add split_pipe function

Open jkisse opened this issue 2 years ago • 7 comments

This is a new toolbox function to split a pipe into two parts

jkisse avatar Feb 27 '23 10:02 jkisse

Codecov Report

Patch coverage: 95.45% and project coverage change: +0.03 :tada:

Comparison is base (cf96a09) 85.06% compared to head (63c66e3) 85.10%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #521      +/-   ##
===========================================
+ Coverage    85.06%   85.10%   +0.03%     
===========================================
  Files           90       90              
  Lines         6080     6102      +22     
===========================================
+ Hits          5172     5193      +21     
- Misses         908      909       +1     
Impacted Files Coverage Δ
pandapipes/toolbox.py 86.61% <95.45%> (+0.78%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov[bot] avatar Feb 27 '23 10:02 codecov[bot]

Thanks for your feedback. Regarding the last point, passing a std_type to create_pipe_from_parameters is not possible because it will raise a UserWarning

jkisse avatar Mar 03 '23 13:03 jkisse

I think, a discussion on the usage of std_types is not a bad idea, but wouldn't fit into this PR. But could you just add the statement net.pipe.std_type.at[new_idx] = pipe_parameters("std_type") or something similar? That would be a little more persistent...

dlohmeier avatar Mar 03 '23 13:03 dlohmeier

the std_type is now copied from the old pipe, after the new pipe has been created. It uses .loc instead of .at because I hope in the future the function will be developed further so that a list of indices can be passed (if you want to split many pipes at once).

jkisse avatar Mar 03 '23 13:03 jkisse

Let's talk about the usage of standard types in Discussion #523.

jkisse avatar Mar 03 '23 13:03 jkisse

In general, I like this feature. However, some thoughts come to mind that should be tackled (whether in this PR or another one, needs more discussion):

* Currently, you only allow one split point per line, while performing changes inside that function. That also means that if a user has a number of sectioning points on the actual pipe, performing the first split changes the network such that the next splits will not work anymore. Allowing for multiple sectioning points would be beneficial here.

* The coordinates of the split junction are derived from the junction geodata of from and to junctions. However, if pipe geodata exists, this geodata also needs to be split and the junction inserted at the split geodata (can be derived quite easily with shapely's interpolate function).

* Why do you neglect the std_type entry in the new pipe? Couldn't you just copy it as well?

About the second point: Are the junction and pipe geodata not the same anyway, or do you mean cases where one has pipe geodata but no junction geodata?

So for splitting a pipe into several section and having both geodata, it should be possible to calculate the junction geodata via interpolation (current implementation) and then copy it for the pipe geodata, right?

Is Shapely's interpolate function to be preferred over manual interpolation?

EPrade avatar Mar 06 '23 14:03 EPrade

In general, I like this feature. However, some thoughts come to mind that should be tackled (whether in this PR or another one, needs more discussion):

* Currently, you only allow one split point per line, while performing changes inside that function. That also means that if a user has a number of sectioning points on the actual pipe, performing the first split changes the network such that the next splits will not work anymore. Allowing for multiple sectioning points would be beneficial here.

* The coordinates of the split junction are derived from the junction geodata of from and to junctions. However, if pipe geodata exists, this geodata also needs to be split and the junction inserted at the split geodata (can be derived quite easily with shapely's interpolate function).

* Why do you neglect the std_type entry in the new pipe? Couldn't you just copy it as well?

About the second point: Are the junction and pipe geodata not the same anyway, or do you mean cases where one has pipe geodata but no junction geodata?

So for splitting a pipe into several section and having both geodata, it should be possible to calculate the junction geodata via interpolation (current implementation) and then copy it for the pipe geodata, right?

Is Shapely's interpolate function to be preferred over manual interpolation?

The geodata of pipes can be a pathway given as point list (LineString in shapely), so it is not automatically a direct connection between the two connected junctions. In such a case, it would make sense to insert the new junction at the exact position along this pathway. Shapely offers nice functions to do just that, but I cannot tell, whether it is to be preferred wrt. comprehensibility or performance.

dlohmeier avatar Mar 15 '23 11:03 dlohmeier