-
Notifications
You must be signed in to change notification settings - Fork 45
[WIP] First try on CGMES Equipment network import #1965
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?
Conversation
…detection + loads with undefined p0/q0 Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
dbf4d23
to
368b592
Compare
Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
...conversion/src/main/java/com/powsybl/cgmes/conversion/elements/EnergyConsumerConversion.java
Outdated
Show resolved
Hide resolved
if (context.cgmes().hasOnlyEquipmentProfile()) { | ||
return Double.NaN; | ||
} | ||
return 0.0; |
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.
What is the aim of this 0.0
value ?
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.
It is for default values when there is several profiles (not only EQ and boundaries) in the CGMES file
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 we could avoid checking if the input data contains only EQ to use a default value of NaN
.
If we let NaN always as the default value we will be able to identify/report situations where some loads defined in the model have not been properly initialized in the SSH (and the code would be simpler).
I realize this is a change over the previous behaviour, but I think that following our current ability of using validation levels, it would be worth it. It would report the Network as not yet containing a complete set of steady state hypothesis.
What do you think?
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.
Your proposal is the cleanest one for sure, but not maybe the most realistic: many IGM will have a validation level at EQ, whereas we really want to run a load flow on it after import. I can imagine that in the TYNDP files, we are far from a SSH validation level... what do you think ?
I think we could also adjust the |
Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
… + correction Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
Signed-off-by: VEDELAGO MIORA <[email protected]>
This PR is to be completed with other attributes but I'm letting it in its current state for us to agree on the design |
… it has to be set by the user Signed-off-by: VEDELAGO MIORA <[email protected]>
SonarCloud Quality Gate failed. |
double targetV = control.targetValue; | ||
Terminal terminal = parent.findRegulatingTerminal(control.cgmesTerminal); | ||
if (voltageRegulatorOn && terminal == null) { // If regulating terminal null, regulation is made local | ||
context.fixed(controlId, () -> "Unmapped regulating terminal and impossible to find an equivalent terminal. Regulation is made local."); |
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.
When we have a regulation on and a null terminal, I think that the good fix is to discard the regulation. Maybe in an other PR.
targetV = terminal.getVoltageLevel().getNominalV(); | ||
} | ||
if (voltageRegulatorOn && (control.targetValue <= 0.0 || Double.isNaN(control.targetValue))) { | ||
targetV = terminal.getVoltageLevel().getNominalV(); |
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.
Here too: the good fix should be to set voltageRegulatorOn
to false. Maybe in an other PR.
@@ -48,13 +49,21 @@ public Context(CgmesModel cgmes, Config config, Network network) { | |||
// based on existing node-breaker info | |||
nodeBreaker = cgmes.isNodeBreaker() && config.useNodeBreaker(); | |||
|
|||
if (config.getMinimumValidationLevel() == ValidationLevel.EQUIPMENT) { | |||
flowsDefaultValue = Double.NaN; |
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.
Maybe add as comment that it means active and reactive power default value. The fixSsh
could be something more explicit as forceValidationLevelToSteadyStateHypothesis
.
// Null terminal if it has not been defined in CGMES file | ||
Terminal terminal = parent.findRegulatingTerminal(control.cgmesTerminal); | ||
if (terminal == null) { | ||
context.invalid(() -> controlId, () -> "Unmapped regulating terminal."); |
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.
Unmapped really? But here it will be forced as local terminal, no?
Please check if the PR fulfills these requirements (please use
'[x]'
to check the checkboxes, or submit the PR and then click the checkboxes)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'
and skip the restNo
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
CGMES imported networks are always at STEADY_STATE_HYPOTHESIS validation level
What is the new behavior (if this is a feature change)?
CGMES imported networks are at EQUIPMENT level if they only contains equipment-related profiles and boundaries (optional). The current PR only regards flows and generators' regulation. Other PRs are to follow.
Does this PR introduce a breaking change or deprecate an API? If yes, check the following: