PV node: feature #185

[scud-soptim]

Fixes #185

Changes proposed in this PR include:

This PR introduces a dedicated voltage_regulator component to enable voltage-controlled (PV) nodes in the power flow model. The component expects the attribute u_ref and enforces the node voltage magnitude to be regulated to this reference value.

To support this behavior, the mathematical formulation and corresponding Jacobian matrix extensions for PV nodes were implemented. In addition, new validation tests and datasets were created to verify correct PV-node behavior across relevant scenarios. The user-facing documentation for the voltage_regulator was updated accordingly.

• Added new component voltage_regulator (requires attribute u_ref) to introduce PV-node behavior.
• Implemented PV-node voltage regulation constraint and integrated it into newton-raphson solver workflow.
• Extended the Jacobian matrix formulation to support PV-node equations consistently.
• Added dedicated validation tests and new test datasets to cover PV-node behavior.
• Updated documentation for voltage_regulator to reflect usage, parameters, and expected behavior.

Fixes

Accounting for shunt elements in the calculation of source results.

Follow-Up Tasks

• Q-limit handling with PV→PQ switching and water-filling–based reactive power distribution among generators.

[scud-soptim]

We do not have sufficient permissions to set the label on this repository.

[figueroa1395]

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]

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.

[mgovers]

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.

[scud-soptim]

We find to out ....

[scud-soptim]

We would like to accept Peters suggestion " • We can work on linking the component to the mathematical model / topology"

[frie-soptim]

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.

[figueroa1395]

We'll look into this and come back to you.

[mgovers]

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

[scud-soptim]

we have to find out...

[figueroa1395]

I'm done reviewing the whole PR. Some additional side remarks:

• When q limits 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 add q limits 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_ref in the same node, different u_ref, with both a/sym_gen a/sym_load_gen, etc., in addition to the validation tests. Moreover, unit tests of other methods besides newton_raphson raising (was this behavior agreed upon already?) when voltage_regulators are present are also welcome (I believe the last can be easily done in the params.json by including the other calculation methods and marking them as expected to fail).

[figueroa1395]

Would it be possible to share, for instance in the issue or elsewhere, a snippet of the more detailed math? I would appreciate that.

[nitbharambe]

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.

[scud-soptim]

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.

[scud-soptim]

I'm done reviewing the whole PR. Some additional side remarks:

• When q limits 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 add q limits 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_ref in the same node, different u_ref, with both a/sym_gen a/sym_load_gen, etc., in addition to the validation tests. Moreover, unit tests of other methods besides newton_raphson raising (was this behavior agreed upon already?) when voltage_regulators are present are also welcome (I believe the last can be easily done in the params.json by including the other calculation methods and marking them as expected to fail).

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
This PR is intentionally limited to introducing PV nodes and their basic handling. Q-limit enforcement (including PV→PQ switching, cooldown, and hysteresis to avoid oscillations) is considered a separate concern and should, in my view, be handled independently to keep responsibilities and changes clearly separated.

Q-limits / cooldown / hysteresis
I fully agree that cooldown / hysteresis is essential once Q-limits are enforced. To keep concerns cleanly separated, it would be great to track this as a dedicated issue and follow-up PR. If you are open to that, you could create a separate issue for Q-limit handling and assign it to us.

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
Regarding the interaction of automatic tap changers and voltage regulators: having both acting on the same bus is, from a modeling perspective, not well-defined. A tap changer can modify the bus voltage via the transformer, while a voltage regulator enforces a fixed voltage setpoint at that same bus, which can lead to conflicting control objectives.

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.

[nitbharambe]

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.

[scud-soptim]

Goedemorgen everyone :-),
I hope you’re doing well. I have a question regarding the process of resolving pull request comments. How do you usually approach this? And who is responsible for marking a discussion as resolved?

[figueroa1395]

Goedemorgen everyone :-), I hope you’re doing well. I have a question regarding the process of resolving pull request comments. How do you usually approach this? And who is responsible for marking a discussion as resolved?

Good morning @scud-soptim! As reviewers, we'll go over the comments + commits and mark them as resolved once addressed. However, if we miss any comments, please don't hesitate to ping us and we'll address them as soon as possible.

[figueroa1395]

Scope of this PR
This PR is intentionally limited to introducing PV nodes and their basic handling. Q-limit enforcement (including PV→PQ switching, cooldown, and hysteresis to avoid oscillations) is considered a separate concern and should, in my view, be handled independently to keep responsibilities and changes clearly separated.

Q-limits / cooldown / hysteresis
I fully agree that cooldown / hysteresis is essential once Q-limits are enforced. To keep concerns cleanly separated, it would be great to track this as a dedicated issue and follow-up PR. If you are open to that, you could create a separate issue for Q-limit handling and assign it to us.

The issue has been created and assigned to you. See #1236 .

Let's indeed keep the scope of this PR as small as possible and do incremental improvements.

Interaction between tap changers and voltage regulators
Regarding the interaction of automatic tap changers and voltage regulators: having both acting on the same bus is, from a modeling perspective, not well-defined. A tap changer can modify the bus voltage via the transformer, while a voltage regulator enforces a fixed voltage setpoint at that same bus, which can lead to conflicting control objectives.

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.

You are right: let's not deal with this issue for now. Can you throw a hard error/crash in the core when automatic tap regulation is used in combination with voltage regulators? In addition, can you also add this check to the validation function.

On the other hand, I've resolved several comments and made a few additional minor ones.

[figueroa1395]

We'll look into this and come back to you.

[nitbharambe]

The imag(output.laod_gen[load_gen].s) should be made zero here additionally if that load_gen is regulated. That value corresponds to q_specified for the regulated load and it should be discarded.

[frie-soptim]

For a bus the functions calculate_load_gen_result, calculate_voltage_regulator_result and calculate_source_result are called.
I left the function calculate_load_gen_result unchanged, because if there are no regulators, then it already yields the correct value.
If there are regulators, then it yields q_specified at first, but in calculate_voltage_regulator_result, it calculates a new Q value and sets an updated S (={prev P, new Q}) and recalculates the current I. Thus, the load_gen always has the final S value, without having to look at the regulator results. The regulator result then shows the change relative to the specified value.

Is that not the desired outcome?

If I set S to 0 in calculate_load_gen_result for loads/gens with a regulator, then I would have to do the complex regulator/appliance status check that I am doing in calculate_voltage_regulator_result anyway.

[nitbharambe]

We had originally intended for q_specified to be ignored for a regulated generator, and a regulated generator would contain only the Q provided by regulation. In the current implementation, the generator can have a fixed Q provided by q_specified + a variable Q offset supplied by the virtue of regulation.

Is that not the desired outcome?

We would ask you back instead. Can you confirm if this is intentional? And would you then be using a generator with a regulator with a non-zero q_specified for your calculations? Trying to understand use cases from a user perspective here. If yes, can you add this behavior explicitly to the documentation?

The original proposal and current implementation will behave the same if you only have one PV generator per bus. The q_specified will be completely ignored for both power flow equation and post processing. The real difference will show up when you have more than one PV generator per bus. Then we need to divide the Q injection from power flow equation per bus to multiple PV generators. The question is how to divide. The original proposal just divides the Q absolute injection equally and ignores the q_specified. The current implementation, however, uses q_specified as the starting point. We only divide the remaining Q, namely regulator offset, equally to all the PV generators.

To demonstrate,
Input grid:

source -- node -- line -- node -- gen+regulator_1 (p_specified_1, q_specified_1)
                                                 -- gen+regulator_2 (p_specified_2, q_specified_2)

gen_output_1_p = p_specified_1
gen_output_2_p = p_specified_2

Original proposal:
gen_output_1_q = q_remaining/2
gen_output_2_q = q_remaining/2
(where q_remaining = bus_injection)

Current implementation:
gen_output_1_q = q_specified_1 + q_remaining/2
gen_output_2_q = q_specified_2 + q_remaining/2
(where q_remaining = bus_injection - q_specified_1 - q_specified_2)

Also consider similar implications for q_min and q_max implementation.

[figueroa1395]

Some additional remarks:

• We'll remove the feature flag after the holidays. That way both parties can implement + test more easily and we start adding immediate value for users in general.
• Can you provide validation tests in which multiple load_gens of different types (const_impedance, const_current, and const_power) are present?
• Can you also provide validation tests for a simple grid in which a regulated and a non-regulated sym_load are at the same node?

[figueroa1395]

This is a question for you as users.

Should we make limit_violated an enum instead? With values such as PV_operation, PQ_operation or something similar (open to suggestions)? Should we add if the lower or upper bound was violated? Or just keep as is? What is your use case?

[figueroa1395]

We don't allow voltage regulators at a node where a source is also present, since it is not clear how to handle this logic (node with source + load + regulator); for this reason, all the source related logic in this check can be removed.

On the other hand, please make a separate check + crash for this (node with source + regulator is forbidden), with unit tests and documentation.

[figueroa1395]

Let's split this check into a separate one. This one can be done at optimizer.hpp

Edit: I overlooked the above recommendation, which is actually wrong. The correct place for this check would be in tap_position_optimizer.hpp at optimize which is the entry point (e.g. at

power-grid-model/power_grid_model_c/power_grid_model/include/power_grid_model/optimizer/tap_position_optimizer.hpp

Line 911 in 90dbe80

auto optimize(State const& state, CalculationMethod method) -> MathOutput<ResultType> final {

)

[figueroa1395]

The maps in this function are likely not needed and we can go via the topology instead. I will look at a better way to implement this and get back to you.

This is related to https://github.com/PowerGridModel/power-grid-model/pull/1220/changes#r2627765403

[nitbharambe]

Additionally, we propose following:
To clearly distinguish between const_pq load and a regulated load, we can introduce an additional member in LoadGenType enum: const_pv.
Allowing a generator with for example LoadGenType.const_impedance / LoadGenType.const_current might be impractical (correct me if not) and unintuitive to the user when compared to existing functionality.
The present implementation can stay as is with an extra fallthrough to const_pq at all places of switch statement for LoadGenType. An error is to be raised if a regulator is unavailable for const_pv type of load or other LoadGenType is coupled with a regulator component.
This will also help de-complicate the linking logic as we have the exact type available at the component creation stage. (which we are going to be addressing in the later PR or in scud-soptim#1). (cc. @figueroa1395 )