NonReturnValveController created, test in pipeflow_internals added and docu updated #118
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #118 +/- ##
===========================================
+ Coverage 91.22% 91.43% +0.21%
===========================================
Files 62 61 -1
Lines 3303 3294 -9
===========================================
- Hits 3013 3012 -1
+ Misses 290 282 -8
Continue to review full report at Codecov.
|
| j = 0 | ||
| for i in self.element_index: | ||
| self.v_m_per_s.append(self.net.res_valve.loc[i, "v_mean_m_per_s"]) | ||
|
|
||
| if self.net.valve.loc[i, "opened"] and self.v_m_per_s[j] < 0: | ||
| # use the element indices, where opened = True, otherwise NaN would be in self.v_m_per_s | ||
| self.net.valve.loc[i, "opened"] = False | ||
| j += 1 |
There was a problem hiding this comment.
This could be solved without a loop which would make it much more performant! Should me something like:
rel_valve = self.net.valve.loc[self.element_index, :]
rel_valve.loc[rel_valve["opened] and rel_valve['v_mean_m_per_s']<0, 'opened] = False
Furthermore, I think, in the initial step you should only do one thing: Open all non return valves. Following reason: Right now I don't think the controller acts the way it should. Imagine following thing. Right now you only consider the valves which are open. But if you run it once, you close all valves which have a v_mean <0. Now you change something in your grid and run it again. Now, the valves you closed in the first run are not considered anymore. However, I think they still should. I would rather do it like this: All valves covered in the element_index should be considered.
| for i in range(len(self.element_index)): | ||
| if self.net.valve.loc[self.element_index[i], "opened"] and self.v_m_per_s[i] < 0: | ||
| return False |
There was a problem hiding this comment.
Same as above: converged = np.all(self.net.valve.loc[self.element_index, 'v_mean_m_per_s')>=0)
| j = 0 | ||
| for i in self.element_index: | ||
| self.v_m_per_s.append(self.net.res_valve.loc[i, "v_mean_m_per_s"]) | ||
|
|
||
| if self.net.valve.loc[i, "opened"] and self.v_m_per_s[j] < 0: | ||
| # use the element indices, where opened = True, otherwise NaN would be in self.v_m_per_s | ||
| self.net.valve.loc[i, "opened"] = False | ||
| j += 1 |
There was a problem hiding this comment.
Another thing what I was thinking about: I would return to the initial state, meaning all valves should have the same values before you used the controller. Meaning, if the user set some non return valves on open and you closed the now, I would in a finalizing step return to a open valve, as only if you really want to use the controller the controller should also have an effect.
| # just calling init of the parent | ||
| super().__init__(net, in_service=in_service, recycle=recycle, order=order, level=level, | ||
| drop_same_existing_ctrl=drop_same_existing_ctrl, | ||
| matching_params=matching_params, initial_powerflow=initial_pipeflow, |
There was a problem hiding this comment.
we changed the attribute from initial_powerflow to initial_run. Please update your pandapower and pandapipes version!
| if set_q_from_cosphi: | ||
| logger.error("Parameter set_q_from_cosphi deprecated!") | ||
| raise ValueError |
There was a problem hiding this comment.
We don't have q-values in pandapipes. Q-values stands for reactive power.
| Saving the user-defined values, determine valves with negative flow velocities, | ||
| set opened to False for these. | ||
| """ | ||
| pp.pipeflow(self.net, self.kwargs) |
There was a problem hiding this comment.
Don't do a pipeflow within a controller. Do it with the flag initial_run
No description provided.