pandapipes icon indicating copy to clipboard operation
pandapipes copied to clipboard

Water Tower Component

Open SimonRubenDrauz opened this issue 4 years ago • 3 comments

Introducing new component: Water Tower

SimonRubenDrauz avatar Aug 03 '20 08:08 SimonRubenDrauz

Codecov Report

Merging #107 into develop will increase coverage by 0.42%. The diff coverage is 90.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #107      +/-   ##
===========================================
+ Coverage    90.67%   91.09%   +0.42%     
===========================================
  Files           59       60       +1     
  Lines         3248     3224      -24     
===========================================
- Hits          2945     2937       -8     
+ Misses         303      287      -16     
Impacted Files Coverage Δ
pandapipes/create.py 97.60% <75.00%> (ø)
pandapipes/create_toolbox.py 100.00% <100.00%> (ø)
pandapipes/std_types/std_type_toolbox.py 91.30% <0.00%> (-3.94%) :arrow_down:
pandapipes/std_types/std_type.py 81.94% <0.00%> (-2.12%) :arrow_down:
pandapipes/io/file_io.py 81.25% <0.00%> (+1.70%) :arrow_up:
pandapipes/io/io_utils.py 78.08% <0.00%> (+9.33%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e208959...e044b08. Read the comment docs.

codecov[bot] avatar Aug 03 '20 08:08 codecov[bot]

To me it is a bit questionable whether this component makes sense in this form, especially due to the similarity to the ext_grid that @jkisse mentioned. There are several issues connected to this component:

  1. Why is it so important to create a whole component just for the sake of one equation that can easily be implemented by a user himself? If this is implemented as a controller with an ext_grid, it is possible to adapt the pressure of the ext_grid with the help of the water height, and you could even implement a control loop that adapts the average height during a time series simulation based on the water extraction.
  2. If you implement this as a component, what do you do if an ext_grid is connected to the same junction as a water tower and they don't show the same pressure? What you implemented by now only divides the mass flows over those components in the results, but the pressure PINIT can only be set in each component individually, so the latest initialized component determines the pressure. Maybe this is the time where we should think about how to aggregate different components and their behavior in one function, which would also lead to a speed-up if e.g. all pipes and valves can be calculated as one part of the pit. Yet, this adds more complexity to our whole structure which is already extremely complex.

dlohmeier avatar Aug 05 '20 08:08 dlohmeier

Thanks for your feedback. I adapted now everything. What do you think?

SimonRubenDrauz avatar Aug 10 '20 12:08 SimonRubenDrauz