-
Notifications
You must be signed in to change notification settings - Fork 47
PV node: feature #185 #1220
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
PV node: feature #185 #1220
Conversation
Signed-off-by: Eduard Fried <eduard.fried@soptim.de> # Conflicts: # power_grid_model_c/power_grid_model/include/power_grid_model/math_solver/common_solver_functions.hpp
… bus injections onto pv generators Signed-off-by: Eduard Fried <eduard.fried@soptim.de> # Conflicts: # power_grid_model_c/power_grid_model/include/power_grid_model/math_solver/common_solver_functions.hpp
…urce - set pq load gens from input - for pv gens set P (from input) and distribute Q (from bus injection) - push rest of P and Q to source - if both PV gen and source conntected to a bus, source will only get P Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
…nerators Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
…r type to const_pq Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
- set delta q to zero at pv bus for all load/generators, not just pv generators Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
- remove previous implementation with the const_pv value Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
- add u_ref - add q_min and q_max, as they may be dependent on the specified active power value of the generator Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
|
We do not have sufficient permissions to set the label on this repository. |
Thanks for the contribution! I have added the necessary label to the PR. We'll proceed to review it as soon as possible. |
mgovers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @scud-soptim, Nice to see this coming together the way it is.
I've gone through most of the interface and boilerplate code. I'll continue with the review of the actual solver and input/output conversions on Monday, but here is already some input.
| | name | data type | unit | description | | ||
| | ----------------- | ----------------- | -------------------------- | -------------------------------------------------------------------- | | ||
| | `q` | `RealValueOutput` | volt-ampere-reactive (var) | reactive power provided by the voltage regulator | | ||
| | `limit_violated` | `int8_t` | - | reactive power limit violation indicator (not yet fully implemented) | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated to this PR but maybe we should add such a flag to the tap regulator as well. Relates to #1215
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment does not relate to SOPTIM
| When fully implemented, if the reactive power constraints are violated, the generator will operate at the limit and the node becomes a PQ node: | ||
|
|
||
| $$ | ||
| \begin{eqnarray} | ||
| & P_{\text{gen}} = P_{\text{specified}} \\ | ||
| & Q_{\text{gen}} = Q_{\text{min}} \text{ or } Q_{\text{max}} \\ | ||
| & |U_{\text{node}}| = \text{calculated from power flow} | ||
| \end{eqnarray} | ||
| $$ | ||
|
|
||
| In this case, `limit_violated` will indicate which limit was exceeded, and the actual voltage at the node may differ from `u_ref`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this is not fully implemented yet, do we want to explicitly mention the current behavior with a big warning note that this is temporary behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively, you may add this as an experimental feature temporarily. That way, it's fine if things will change but we can already merge it and it can already be tested by users, but we do not settle on final results yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, could you describe how to implement an experimental feature - otherwise we would prefer a warn-message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a warning-note!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a function check_no_experimental_features_used in main_model_impl.hpp.
See e.g. #1062 in which we removed the experimental feature flag for current sensors: ( https://github.com/PowerGridModel/power-grid-model/pull/1062/changes#diff-cc6d6a55667a569640705486e5990498b32f2a3ecf9fec770922a5b69a71920a )
You can add a similar check that searches for voltage regulators in power flow calculations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using experimental feature
| def validate_voltage_regulator(data: SingleDataset) -> list[ValidationError]: | ||
| errors = validate_base(data, ComponentType.voltage_regulator) | ||
| errors += _all_valid_ids(data, ComponentType.voltage_regulator, "regulated_object", [ | ||
| ComponentType.sym_gen, | ||
| ComponentType.asym_gen, | ||
| ComponentType.sym_load, | ||
| ComponentType.asym_load | ||
| ]) | ||
| errors += _all_boolean(data, ComponentType.voltage_regulator, "status") | ||
| errors += _all_unique(data, ComponentType.voltage_regulator, "regulated_object") | ||
| errors += _all_greater_than_zero(data, ComponentType.voltage_regulator, "u_ref") | ||
| return errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add a unit test for this function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| RealValue<sym> q{}; | ||
|
|
||
| // provide generator info, as the regulator component has no other access to it | ||
| ID generator_id{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you probably want to provide the generator math idx, not the user-specified ID.
This is also how we produce output for, e.g., branch voltages: the branch knows via the branch idx the index of the node for each end, and then it uses the voltage from the node solver output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We find to out ....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We would like to accept Peters suggestion " • We can work on linking the component to the mathematical model / topology"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use the generator_id in the output_result result function.
if (load_gen_math_id.group != -1) {
// is voltage regulator always in same group as the generator it regulates?
for (auto const& vr_output: math_output.solver_output[load_gen_math_id.group].voltage_regulator) {
if (vr_output.generator_id == voltage_regulator.regulated_object()) {
return voltage_regulator.template get_output<sym>(vr_output);
}
}
}
Unfortunately, i am not familiar enough with the internal topology structures to easily work without this id.
| // TODO: #185 There should be at most one voltage regulator per load_gen. Is there a simpler way to represent this? | ||
| DenseGroupedIdxVector voltage_regulators_per_load_gen; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current way is OK.
The way that DenseGroupedIdxVector works is that it already uses an efficient representation, so both memory-wise and when looping over the data, it optimizes for space.
In particular, rather than storing something along the lines of {load_gen_1: [regulator_1], load_gen_2: [], load_gen_3: [], load_gen_4: [regulator_2]}, it stores it like {"regulator_1": [load_gen_1], "regulator_2: [load_gen_4]}
C++23 std::optional should be able to optimize this code even further, but we haven't gotten to that yet.
P.s.: we welcome better naming conventions!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we remove the TODO Statement.
|
|
||
| namespace power_grid_model { | ||
|
|
||
| class VoltageRegulator : public Regulator { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add unit tests for this class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added unit test
| // getter | ||
| double injection_direction() const { return injection_direction_; } | ||
| double u_ref() const { return u_ref_; } | ||
| double q_min() const { return q_min_; } | ||
| double q_max() const { return q_max_; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make constexpr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| transformer_tap_regulator_8 | ||
| ``` | ||
|
|
||
| ### Voltage Regulator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably good to explicitly mention that only Newton-Raphson calculation method is supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
docs/user_manual/components.md
Outdated
| If these limits are not provided, the reactive power can take any value needed to maintain the voltage setpoint. | ||
|
|
||
| ```{note} | ||
| The `regulated_object` must reference a generator (`sym_gen` or `asym_gen`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code including the data validator also supports sym_load and asym_load
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| return state.topo_comp_coup->load_gen[obj_seq]; | ||
| }(); | ||
| if (gen_math_id.group != -1) { | ||
| // is voltage regulator always in same group as the generator it regulates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes because the group represents the connected component (in the graph-theoretical sense) it belongs to. That is, if you have 2 distinct subgrids that are not connected to eachother, then you will have 2 topological groups, and therefore also 2 math solvers (one for each subgrid).
As a special case, if a subgrid does not contain a source, the subgrid is considered disconnected, will not have an associated math solver, and therefore, the group for all components in that subgrid will be -1
if the groups are not equal, there's a bug in the code concerning topology
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to find out...
figueroa1395
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm done reviewing the whole PR. Some additional side remarks:
- When
qlimits are fully implemented, don't forget the "cooldown" factor to avoid back and forth between PV and PQ nodes. In addition, don't forget to addqlimits to the tests as well. - Don't forget to check whether power flow with automatic tap regulators work seamlessly with PV nodes now. Probably some unit + validation test (if it works) are a good idea.
- It would be nice to have some unit tests on combinations of having multiple regulators with the same
u_refin the same node, differentu_ref, with botha/sym_gena/sym_load_gen, etc., in addition to the validation tests. Moreover, unit tests of other methods besidesnewton_raphsonraising (was this behavior agreed upon already?) whenvoltage_regulators are present are also welcome (I believe the last can be easily done in theparams.jsonby including the other calculation methods and marking them as expected to fail).
| explicit VoltageRegulator(VoltageRegulatorInput const& voltage_regulator_input, | ||
| ComponentType regulated_object_type, | ||
| double injection_direction) | ||
| : Regulator{voltage_regulator_input, regulated_object_type}, | ||
| injection_direction_{injection_direction}, | ||
| u_ref_{voltage_regulator_input.u_ref} {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this is to be done when the reactive power limit is fully implemented, but I'll mention it anyway: q_min and q_max were not initialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| q_max_ = new_q_max; | ||
| changed = true; | ||
| } | ||
| return changed; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Why do we need to return changed? Why do we need changed in the first place? Same for set_u_ref.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed not needed, removed
| .q_min = RealValue<sym>{q_min_}, // TODO: #185 divide by 3 for asymmetric case? | ||
| .q_max = RealValue<sym>{q_max_}, // TODO: #185 divide by base_power? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I believe dividing by
base_poweris indeed the way to go, as we do out calculations internally inpu. - Dividing by 3 for asymmetric doesn't sound right, but keeping as
RealValue<sym>{q_m*_}isn't right either. If you keep as is, it means that each phase supportsq_m*which in total makes it3 q_m*; if you divide by three, that means each phase supports exactlyq_m* / 3which doesn't make sense as it should be possible have50 pu, 30 pu, 20 pufor phasesa, b, crespectively if the total allowed is100 pu q_m*for example. - That said, it's probably a good idea to just keep these limits as
doubles and manage internal logic elsewhere?
@nitbharambe Please corroborate that what I'm stating isn't incorrect, as EE isn't my field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this attribute is in p.u. internally so both can be divided by base_power_3p.
These variables here can denote total limit, ie. sum of all phases power.
Further division by 3 would depend on whether loadgen is symmetric or asymmetric type. Let's handle that at either param calculation or solver side where we have information on the regulated object.
| template <symmetry_tag sym, IntSVector(PowerFlowInput<sym>::*component), class Component> | ||
| void prepare_input_status(main_model_state_c auto const& state, std::vector<Idx2D> const& objects, | ||
| std::vector<PowerFlowInput<sym>>& input) { | ||
| for (Idx i = 0, n = narrow_cast<Idx>(objects.size()); i != n; ++i) { | ||
| Idx2D const math_idx = objects[i]; | ||
| if (math_idx.group == isolated_component) { | ||
| continue; | ||
| } | ||
| (input[math_idx.group].*component)[math_idx.pos] = | ||
| main_core::get_component_by_sequence<Component>(state.components, i).status(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation is identical to the one for StateEstimationInput<sym>::*component. You can unify both by adding a concept to support both IntSVector(PowerFlowInput<sym>::*component) and IntSVector(StateEstimationInput<sym>::*component) as valid template parameters.
| Idx const obj_seq) { | ||
| using sym = typename SolverOutputType::sym; | ||
|
|
||
| Idx2D const gen_math_id = [&]() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: When referring to a generator here (gen) can you instead refer to a load/generator (load_gen), or something along those lines? To stay consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C.f.r. https://github.com/PowerGridModel/power-grid-model/pull/1220/changes#r2620017314. This method shouldn't support voltage regulators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C.f.r. https://github.com/PowerGridModel/power-grid-model/pull/1220/changes#r2620017314. This method shouldn't support voltage regulators.
| // expect that one load/gen is only regulated by one regulator and if multiple loads/gens with regulators | ||
| // are connected to the same bus, they all have the same u_ref |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a unit test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| std::map<Idx, Idx> loadgen_to_regulator{}; | ||
| for (auto const& [load_gen, regulators] : enumerated_zip_sequence(*this->voltage_regulators_per_load_gen_)) { | ||
| if (input.load_gen_status[load_gen] == 0) { continue; } | ||
| for(Idx const regulator_number : regulators) { | ||
| if (input.voltage_regulator[regulator_number].status != 0) { | ||
| loadgen_to_regulator.insert({load_gen, regulator_number}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: Can you make this into a separate function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| // PV-bus voltage identity row: | ||
| // For PV buses, the Q-equation is replaced by the algebraic constraint | ||
| // rPV = V - Vset = 0. | ||
| // Since the state vector uses relative voltage increments ΔV_rel = ΔV / V, | ||
| // the derivative ∂rPV/∂ΔV_rel equals the current voltage magnitude V. | ||
| // Therefore: | ||
| // - all off-diagonal voltage derivatives (L block) are zero, | ||
| // - all θ-derivatives (M block) are zero, | ||
| // - only the diagonal L entry is set to V. | ||
| // This removes the Q-equation and enforces the PV voltage constraint in the NR step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to share, for instance in the issue or elsewhere, a snippet of the more detailed math? I would appreciate that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we directly start from u_ref and ignore Q equation by setting full row=0, delta U will be 0 throughout PV behaviour. What would be the purpose of setting L=v specifically?
The jacobian diagonal can also be set to 1.0 instead. Equivalent behaviour but descriptive that we are ignoring this whole row.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right that, numerically, setting the Jacobian diagonal to 1.0 would lead to equivalent behavior in this formulation: since the PV row replaces the Q equation and we start from U = U_ref, the Newton update keeps ΔU = 0 as long as the bus remains PV.
The reason for explicitly setting the PV row to L = v (and zeroing the rest of the row) is mainly semantic and robustness-related: it makes it explicit in the Jacobian that this equation is an identity constraint enforcing V = Vset, not a physical power balance. This becomes important for readability, debugging, and for later extensions (e.g. PV→PQ switching, Q-limit handling), where it is useful to clearly distinguish “ignored equations” from active ones.
So while diag = 1.0 is mathematically sufficient, the current form is intentionally descriptive rather than minimal.
Thanks a lot for the detailed review and the additional remarks — they are all valid and appreciated. I would like to clarify scope first and then address the individual points: Scope of this PR Q-limits / cooldown / hysteresis For context: I have already implemented Q-limit handling with active-set–style PV→PQ switching and cooldown/hysteresis in another power-flow project, so the necessary design and implementation experience is available once this topic is picked up. Interaction between tap changers and voltage regulators Rather than only testing this combination, it would therefore be preferable to introduce a validation check that detects and rejects configurations where a tap changer and a voltage regulator operate on the same node. |
| state, state.topo_comp_coup->voltage_regulator, pf_input, | ||
| [&state](Idx i) { return state.comp_topo->regulated_object_type[i] == ComponentType::generic_load_gen; }); | ||
|
|
||
| prepare_input_status<sym, &PowerFlowInput<sym>::load_gen_status, GenericLoadGen>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine the prepare_input filters out status=0 elements. So the resulting loadgen which is available at the newton raphson solver should contain only status=1 elements. Since similar logic gets followed for voltage regulators, only status=1 elements should be present there. So I imagine further checks on status would not be necessary.
We might have to somehow add an additional check to exclude voltage regulators from input to ignore the voltage_regulator if regulated object is disabled. I think we should discuss and provide a detailed way around this @mgovers / @figueroa1395 .
Will add further details.
Unrelated to PR comment for maintainers:
- We should rename
prepare_inputtoprepare_input_status_param. Maybe we should split out predicate logic intoprepare_input_status_param_with_predicate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it makes sense. definitely out of scope for this PR but let's have a brainstorm session on this in the new year
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Let's schedule a discussion about this.
| get_component_sequence_offset<Regulator, Component>(state.components); | ||
| } | ||
|
|
||
| template <std::same_as<VoltageRegulator> Component, class ComponentContainer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be combined with above function via changing std::derived_from<Regulator>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| const BusType bus_type = bus_types_[row]; | ||
| for (Idx k = indptr[row]; k != indptr[row + 1]; ++k) { | ||
| Idx const j = indices[k]; | ||
| if (bus_type == BusType::pv) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The if(bus_type == BusType::pv) can be moved outside of the column loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
|
||
| // PV-bus voltage identity row: | ||
| // For PV buses, the Q-equation is replaced by the algebraic constraint | ||
| // rPV = V - Vset = 0. | ||
| // Since the state vector uses relative voltage increments ΔV_rel = ΔV / V, | ||
| // the derivative ∂rPV/∂ΔV_rel equals the current voltage magnitude V. | ||
| // Therefore: | ||
| // - all off-diagonal voltage derivatives (L block) are zero, | ||
| // - all θ-derivatives (M block) are zero, | ||
| // - only the diagonal L entry is set to V. | ||
| // This removes the Q-equation and enforces the PV voltage constraint in the NR step. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we directly start from u_ref and ignore Q equation by setting full row=0, delta U will be 0 throughout PV behaviour. What would be the purpose of setting L=v specifically?
The jacobian diagonal can also be set to 1.0 instead. Equivalent behaviour but descriptive that we are ignoring this whole row.
| .q_min = RealValue<sym>{q_min_}, // TODO: #185 divide by 3 for asymmetric case? | ||
| .q_max = RealValue<sym>{q_max_}, // TODO: #185 divide by base_power? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this attribute is in p.u. internally so both can be divided by base_power_3p.
These variables here can denote total limit, ie. sum of all phases power.
Further division by 3 would depend on whether loadgen is symmetric or asymmetric type. Let's handle that at either param calculation or solver side where we have information on the regulated object.
| ComponentList<Node, Line, AsymLine, Link, GenericBranch, Transformer, ThreeWindingTransformer, Shunt, Source, | ||
| SymGenerator, AsymGenerator, SymLoad, AsymLoad, SymPowerSensor, AsymPowerSensor, SymVoltageSensor, | ||
| AsymVoltageSensor, SymCurrentSensor, AsymCurrentSensor, Fault, TransformerTapRegulator>; | ||
| AsymVoltageSensor, SymCurrentSensor, AsymCurrentSensor, Fault, TransformerTapRegulator, VoltageRegulator>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also Nice to add a check in dependent_type_check at power_grid_model_c/power_grid_model/include/power_grid_model/main_core/main_model_type.hpp
In this case it should be dependent_type_check<CompList, VoltageRegulator, SymGenerator, AsymGenerator, SymLoad, AsymLoad> alongside dependent checks for other components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
…ure for now - until PV->PQ conversion is implemented Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
…_solver - calculate calculate_pf_result() function need access to voltage regulators -> read them directly from y_bus, instead of passing them via function arguments - remove references to voltage regulators (and shunts) that were passed on to the result() function in other solvers Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Signed-off-by: Eduard Fried <eduard.fried@soptim.de>
Fixes #185
Changes proposed in this PR include:
voltage_regulator(requires attributeu_ref) to introduce PV-node behavior.voltage_regulatorto reflect usage, parameters, and expected behavior.Fixes
Follow-Up Tasks