fix(3ph): report missing zero sequence parameters with a descriptive error - #3089
fix(3ph): report missing zero sequence parameters with a descriptive error#3089Houmgaor wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3089 +/- ##
===========================================
+ Coverage 72.12% 72.14% +0.02%
===========================================
Files 355 355
Lines 39378 39410 +32
===========================================
+ Hits 28400 28431 +31
- Misses 10978 10979 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
CI note: the two red checks, |
|
|
||
| **INPUT**: | ||
| **net** - The pandapower format network | ||
| """ |
There was a problem hiding this comment.
Please use google style docstring.
I am aware that pandapower is currently a mix of different doc string styles, but longterm everything should be using the google style docstring, as this has been the best version for rendering in read the docs so far.
There was a problem hiding this comment.
I followed the style in the rest of the file, I can change for this function only but that will be inconsistent with the rest of the file.
There was a problem hiding this comment.
The file only has one other docstring using this style, please do convert the docstring to google style even if it is inconsistent with the other docstring. These are internal use functions anyway and will never appear in the docs. I would rather have this already in the style that should be applied to all of pandapower than having to convert it later at some point.
| if parameter not in net[element].columns: | ||
| missing.setdefault(element, {})[parameter] = example_value | ||
| elif net[element][parameter].isnull().values[is_element[element]].any(): | ||
| incomplete.setdefault(element, []).append(parameter) |
There was a problem hiding this comment.
There is a function for checking the existance of a column in the table, it will add the table and column if they do not exist (with default value if available). See pandapower.create._utils.add_column_to_df (on branch release/v4.0.0).
There was a problem hiding this comment.
pandapower.create._utils.add_column_to_df adds a column rather that raising the error. The former may add default values (x0x_max/s_sc_max_mva) and create false short-circuits, so I'd rather raise for the code. Also, the function is only on v4 if you still want to use it, so the PR would need a rebase if you want the feature there.
| _ZERO_SEQUENCE_EXAMPLE_VALUES = { | ||
| "ext_grid": {"s_sc_{case}_mva": "1000", "rx_{case}": "0.1", "x0x_{case}": "1.0", "r0x0_{case}": "0.1"}, | ||
| "line": {"r0_ohm_per_km": "0.1", "x0_ohm_per_km": "0.1", "c0_nf_per_km": "0.0"}, | ||
| } |
There was a problem hiding this comment.
Default Values should not be "hidden" in the code. They should be added to the pandera metadata and used from there.
There was a problem hiding this comment.
I agree with the idea, but metadata is on v4 only. I can rebase.
|
Thanks for the reviews @KS-HTK . I opened the PR against |
|
So this is the part where I will leave the decission to you, new features should be done against It will require a update for release/v4.0.0 at some point, might as well be now if it is not a critical issue. |
…error runpp_3ph and the unbalanced short-circuit calculation need zero sequence parameters that runpp does not. Only s_sc_max_mva and rx_max were checked, the remaining ext_grid and line parameters raised a bare KeyError from pd2ppc_zero. Columns that are present but undefined were not checked at all, so they failed with a TypeError or silently produced NaN results. All of them are now collected in one check that runs before the network is converted, so that they are reported together with an example of how to set them. The example values are read from a new "example" key in the pandera metadata rather than being hardcoded in pd2ppc_zero, so the schema stays the single source of truth for them. Also adds the missing r0x0_min and x0x_min to the ext_grid schema and tags s_sc_max_mva as a 3ph parameter, so that the generated dependency group and the element documentation match what runpp_3ph actually requires. Closes e2nIEE#3069
240d24e to
2105556
Compare
|




Fixes #3069.
runpp_3phand the unbalanced short-circuit calculation need zero sequence parameters thatrunppdoes not. Onlys_sc_max_mvaandrx_maxwere checked; the remainingext_gridandlineparameters raised a bareKeyErrorfrompd2ppc_zero.While working on this I found a third variant that #3069 does not list: when the column exists but holds no value.
create_cigre_network_mv()+add_zero_impedance_parameters(net)is such a case — theCABLE_CIGRE_MVandOHL_CIGRE_MVstandard types carry no zero sequence data, so the columns are created and filled withNone. Ondevelopthat net fails withTypeError: unsupported operand type(s) for *: 'NoneType' and 'float'.Changes
_check_zero_sequence_parameterscollects all the requiredext_gridandlineparameters in one place and reports them together, in the message style pandapower already uses fors_sc_max_mva:The example values are the ones
add_zero_impedance_parametersapplies. Undefined values are reported separately, and only for elements in service, so values that are never read are not required. The check is case aware, so the unbalanced short circuit reports the_minparameters whencase="min".It runs once, before the network is converted, rather than at each call site. Both entry points into the zero sequence network are covered:
_pd2ppc(sequence=0)forrunpp_3phand_pd2ppc_zerofor the short circuit.Schema
Following @KS-HTK's pointer in the issue:
r0x0_minandx0x_mintoext_grid. They were missing entirely, although the PowerFactory converter writes them and the shipped IEEE European LV networks contain them.s_sc_max_mvaandrx_maxas3ph, so the generated dependency group is now exactly whatrunpp_3phrequires:['s_sc_max_mva', 'rx_max', 'r0x0_max', 'x0x_max']. Previously the group omitted the two parameters thatrunpp_3phneeds most.I did not enable the commented out
scand3phdependency checks inline_schema. They are commented out ondevelopand onrelease/v4.0.0, so it looks deliberate, and enabling them would requireg0_us_per_km, which is taggedsc/3phbut is not read by any calculation. Happy to include that if you want it.Verification
New parametrized test in
test_runpp_3ph.pycovering each missing parameter group, the undefined value case, and that out of service elements are exempt.Since this check runs on every unbalanced conversion, I also compared full result tables before and after on real networks:
runpp_3phrunpp_3ph Validation.json(PowerFactory reference)case=maxandcase=minWorst absolute difference across all tables:
0.0, with identical NaN patterns. The loadflow, shortcircuit, converter, networks, api, opf, estimation and toolbox suites pass. No new ruff findings.Target branch
Opened against
developas CONTRIBUTING.md asks, and because the bug affects 3.5.x users today. Note thatdevelopandrelease/v4.0.0diverged at v3.4.0 and neither is a superset (35 commits ondevelopare not onrelease/v4.0.0, 49 the other way), so this will not reach the 4.0 milestone on its own.The code changes cherry-pick onto
release/v4.0.0cleanly; only three spots conflict, all trivial: the changelog, thecimmetadata added toext_grid.py, anddoc/elements/ext_grid_par.csv, which is deleted there since the tables are generated from the schemas bydoc/_exts/save_element_tables.py(which means the schema addition documentsr0x0_min/x0x_minautomatically on 4.0). Just say the word and I will open the companion PR againstrelease/v4.0.0, or retarget this one.