Chore/architecture - #820
Conversation
- bugfix when taking ELEMENT_IDX - bugfix VAR_MASS_SLACK - bugfix for bidirectional calculation missing one variable - not converged hydraulic check if selecting heat - renaming VAR_MASS_SLACK to COUNT_VAR_MASS_SLACK
| def create_pit_node_entries(cls, net, node_pit): | ||
| """ | ||
| Function which creates pit branch entries. | ||
| def register_hydraulic_equations(cls, net, branch_pit, node_pit, sys_idx, registry): |
There was a problem hiding this comment.
I am wondering if we could make this approach a little broader and ask the component to register the required equations (or rerun the simulation, see below) with a keyword desceibing the current simulation mode. Then implement the allowed simulation modes for each component. This way it would be possible to add simulation modes
- in a subclass, the superclass call could be used and additional modes implemented in a simple way
- external code could use this component model without having to rewrite too much code
The mode could be based on a string or a Literal.
| raise NotImplementedError | ||
|
|
||
| @classmethod | ||
| def extract_results(cls, net, options, branch_results, mode): |
There was a problem hiding this comment.
As mentioned in my above comment, calculation modes would make sense, here we already implemented this approach. We could adapt it once a concept is found for the above methods.
| self._check_conflict(eq, self.overrides) | ||
| self.overrides.append(eq) | ||
|
|
||
| def assemble(self, size: int) -> tuple[np.ndarray, np.ndarray, np.ndarray, np.ndarray]: |
There was a problem hiding this comment.
I would love to have some more explicit variable names here. It is quite hard to get an overall understanding with such short names, if you need to think about what they mean in the first place.
| n_r = _cat(n_non_mean, 'rows', np.int32) | ||
| n_c = _cat(n_non_mean, 'cols', np.int32) | ||
| n_d = _cat(n_non_mean, 'data', np.float64) | ||
| n_lr = _cat(n_non_mean, 'load_rows', np.int32) |
There was a problem hiding this comment.
I have the impression that load vector is a little confusing in this context, even more now that we are naming lines and rows as "load correlated". We could use this opportunity to change the name, e.g. to "residual vector" or "right hand side vector" (which is of course a little long and inconvenient). For the matrix, we might consider to just explicitly state that this is the Jacobian matrix, as system matrix would also make sense for the pits (i.e. a matrix holding all system relevant information).
| tn = b_pit[:, IdxBranch.TO_NODE].astype(np.int32) | ||
|
|
||
| # variables | ||
| mdot_col = sys_idx.idx(HydVarEq.MDOTINIT, branch_idx) |
There was a problem hiding this comment.
Am I mislead or is this part of the code more or less the same for many branch components? Should we extract it to an external function?
| tn = b_pit[:, IdxBranch.TO_NODE].astype(np.int32) | ||
|
|
||
| # Zero out branch equation contributions for ctrl_active branches (replaced by PC constraint) | ||
| df_dm[ctrl_active] = 0.0 |
There was a problem hiding this comment.
Maybe the pressure control component is a good example of using more component-specific registered variables. We don't need mdot if control_active or dp if control is inactive; here just mdot_in = mdot_out, which could be formulated in its own equation. I think, even if we do not adapt the overall structure, we could remove the derivative_calculation call and replace it with very simplified equations, i.e. p_in = p_out for control_inactive and p_out=p_set for control_active.
3 participants
No description provided.