Skip to content

Fix short-circuit source impedance scaling - #1547

Open
shin4141 wants to merge 1 commit into
PowerGridModel:mainfrom
shin4141:fix/1352
Open

Fix short-circuit source impedance scaling#1547
shin4141 wants to merge 1 commit into
PowerGridModel:mainfrom
shin4141:fix/1352

Conversation

@shin4141

Copy link
Copy Markdown

Fixes #1352

Changes proposed in this PR include

  • Keep sk based on the connected node's rated voltage and carry an SC-only per-source admittance factor of 1/c from short-circuit input preparation.
  • Apply the scaled source admittance consistently to the Norton matrix/RHS and source-current post-processing, without changing power flow, state estimation, or generic cached-YBus semantics.
  • Add a solid source-bus regression for c=0.95, c=1.0, and c=1.1, and update the 13 directly affected short-circuit golden files. Their JSON structure and non-numeric content are unchanged; the semantic changes are numeric SC results only.

Local verification: focused property 6/6 assertions; all single/batch short-circuit validation 16,239/16,239 assertions; power-flow/state-estimation single/batch counterchecks 12,385/12,385 assertions; full configured CTest 198/198 tests; clang-format and git diff --check pass.

Not run locally: Python pytest/coverage, Linux/Windows compiler matrices, sanitizers, full clang-tidy, or upstream CI.

Could you please pay extra attention to the points below when reviewing the PR

  • The same scaled source admittance is used for matrix assembly, the Norton RHS, and source-current output.
  • Low-voltage minimum scaling (c=0.95) is handled per source, while existing internal solver inputs can retain a neutral scaling of 1.0.
  • The broad golden movement is confined to short-circuit outputs.

Checks

  • If changes are related to CI/CD, are they verified via a manual run on this branch? (Not applicable; no CI/CD changes.)
  • Do you wish to discuss this PR in the bi monthly community meeting? (No.)

Assisted-by: OpenAI Codex (repository analysis, implementation, test design, and local verification).

Signed-off-by: Shin <128954611+shin4141@users.noreply.github.com>
@nitbharambe nitbharambe added the bug Something isn't working label Aug 25, 2026
@nitbharambe

Copy link
Copy Markdown
Member

Thanks @shin4141 We shall review the implementation and the modified tests soon.

@nitbharambe nitbharambe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest looks good!

Comment on lines +102 to +109
static ComplexTensor<sym> source_admittance(YBus<sym> const& y_bus, ShortCircuitInput const& input,
Idx source_number) {
assert(input.source_admittance_scaling.empty() ||
input.source_admittance_scaling.size() == input.source.size());
double const scaling =
input.source_admittance_scaling.empty() ? 1.0 : input.source_admittance_scaling[source_number];
return scaling * y_bus.math_model_param().source_param[source_number].template y_ref<sym>();
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: Could you please move the above comment of calculation_input_preparation regarding the reason to this place?

I would like a explaining docstring here along the lines:
"source admittance is calculated on sk for a specific u_ref for powerflow solvers within PGM.
However for generic short circuit calculation, sk is defined at rated voltage, so its source-equivalent impedance scales with c and the corresponding admittance scales with 1 / c = 1 / cabs(input.source[source_number])"

std::vector<FaultCalcParam> faults;
ComplexVector source; // Complex u_ref of each source
ComplexVector source; // Complex u_ref of each source
DoubleVector source_admittance_scaling; // Multiplier for source y_ref in short-circuit calculations

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe if we just do the cabs(input.source[source_number]) at short_circuit_solver directly, we wont need to add source_admittance_scaling right? Lets remove so if thats the case

std::numeric_limits<double>::infinity()};
auto const run_with_voltage_factor = [&](double voltage_factor) {
ShortCircuitInput input;
input.source = {voltage_factor};

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
input.source = {voltage_factor};
input.source = {{voltage_factor, 0.0}};

Good to be explicit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Source impedance vary with ShortCircuitVoltageScaling

2 participants