-
Notifications
You must be signed in to change notification settings - Fork 43
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
CGMES: remove extension for Control Areas, use IIDM Area #3149
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
private static void createControlArea(CgmesControlAreas cgmesControlAreas, PropertyBag ca) { | ||
String controlAreaId = ca.getId("ControlArea"); | ||
cgmesControlAreas.newCgmesControlArea() | ||
private static void createControlArea(Context context, PropertyBag ca) { |
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 suggest you create an AreaConversion.java class in com.powsybl.cgmes.conversion.elements package and move that piece of code there.
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.
Code moved. Had to make public a class in conversion, related to the mapping of terminals of tie flows
cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/Conversion.java
Show resolved
Hide resolved
context.pushReportNode(CgmesReports.convertingElementTypeReport(reportNode, CgmesNames.CONTROL_AREA)); | ||
network.newExtension(CgmesControlAreasAdder.class).add(); | ||
CgmesControlAreas cgmesControlAreas = network.getExtension(CgmesControlAreas.class); | ||
cgmes.controlAreas().forEach(ca -> createControlArea(cgmesControlAreas, ca)); | ||
cgmes.tieFlows().forEach(tf -> addTieFlow(context, cgmesControlAreas, tf)); | ||
cgmesControlAreas.cleanIfEmpty(); | ||
cgmes.controlAreas().forEach(ca -> createControlArea(context, ca)); | ||
cgmes.tieFlows().forEach(tf -> addTieFlow(context, tf)); | ||
context.popReportNode(); |
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 could create an AreaConversion and a TieFlowConversion class (see comments below) and rely on the convert method here (just like other network elements are converted).
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.
specific classes created
// If no control area exists, create one for the whole network, containing the dangling lines as boundaries, | ||
// but only if the network does not contain subnetworks | ||
long numControlAreas = network.getAreaStream().filter(a -> a.getAreaType().equals("ControlAreaTypeKind.Interchange")).count(); | ||
long numSubnetworks = network.getSubnetworks().size(); |
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.
An int could suffice since size() returns an int anyway.
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
context.ignored("Tie Flow", String.format("Tie Flow %s refers to a non-existing control area", tf.getId("TieFlow"))); | ||
return; | ||
} | ||
String terminalId = tf.getId("terminal"); | ||
Boundary boundary = context.terminalMapping().findBoundary(terminalId, context.cgmes()); | ||
if (boundary != null) { | ||
cgmesControlArea.add(boundary); | ||
area.newAreaBoundary() | ||
.setAc(true) |
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.
Isn't it possible to set the correct value for this boolean here in CGMES import, the same way it is done in the CGMES export?
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. Added a specific unit test.
exportOptions.setExtensions(Collections.emptySet()); | ||
exportOptions.setExtensions(Set.of("cgmesControlAreas")); |
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.
Same here
for (Area area : network.getAreas()) { | ||
area.setInterchangeTarget(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.
Same here
if (control.getValue().toString().equals("netInterchange") | ||
if (control.getValue() != null && control.getValue().toString().equals("netInterchange") |
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.
That part could be removed when the issue of the CGMES export altering the IIDM network will be tackled.
// Also, if a default area was created during export to CGMES, remove it from the expected network | ||
// The actual network will not have it, it is the original network directly exported as IIDM | ||
if (!originalNetworkHasAreas) { | ||
expected.getAreaStream().map(Area::getId).toList().forEach(a -> expected.getArea(a).remove()); | ||
} |
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.
That part could be removed when the issue of the CGMES export altering the IIDM network will be tackled.
RegulatingTerminalMapper.mapForTieFlow(terminalId, context).ifPresent(cgmesControlArea::add); | ||
RegulatingTerminalMapper.mapForTieFlow(terminalId, context) | ||
.ifPresent(t -> area.newAreaBoundary() | ||
.setAc(true) |
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.
Isn't it possible to set the correct value for this boolean here in CGMES import, the same way it is done in the CGMES export?
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
Signed-off-by: Luma <[email protected]>
…is used Signed-off-by: Luma <[email protected]>
Quality Gate passedIssues Measures |
String controlAreaId = p.getId(CgmesNames.CONTROL_AREA); | ||
Area area = context.network().getArea(controlAreaId); | ||
if (area == null) { | ||
context.ignored("Tie Flow", String.format("Tie Flow %s refers to a non-existing control area", p.getId("TieFlow"))); |
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 could use the constant from CgmesNames for more consistency:
p.getId(CgmesNames.TIE_FLOW)
context.ignored("Tie Flow", String.format("Tie Flow %s refers to a non-existing control area", p.getId("TieFlow"))); | ||
return; | ||
} | ||
String terminalId = p.getId("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.
You could use the constant from CgmesNames for more consistency:
p.getId(CgmesNames.TERMINAL)
cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/Conversion.java
Show resolved
Hide resolved
double pTolerance; | ||
if (controlArea.hasProperty("pTolerance")) { | ||
pTolerance = Double.parseDouble(controlArea.getProperty("pTolerance")); |
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.
Now that you have removed the else clause, you can merge the declaration of variable pTolerance
with its initialization.
...onversion/src/main/java/com/powsybl/cgmes/conversion/export/SteadyStateHypothesisExport.java
Outdated
Show resolved
Hide resolved
|
||
String controlAreaId = context.getNamingStrategy().getCgmesId(refTyped(network), CONTROL_AREA); | ||
Area area = network.newArea() | ||
.setAreaType("ControlAreaTypeKind.Interchange") |
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.
ControlAreaTypeKind.Interchange
enum value is used here as well as in:
EquipmentExport
SteadyStateHypothesisExport
ControlAreaEq
CgmesConformity1ModifiedConversionTest
EquipmentExportTest
SteadyStateHypothesisExportTest
I think it would be worth creating a constant in CgmesNames
.
Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
What kind of change does this PR introduce?
Update
What is the current behavior?
Information from CGMES control areas is stored using an extension to the Network Model (
CgmesControlArea
).What is the new behavior (if this is a feature change)?
Information from CGMES control areas is stored using native IIDM objects of class
Area
.CGMES
pTolerance
attribute is stored as a property.The ENTSO-E attribute
IdentifiedObject.energyIdentCodeEic
is stored as an alias.Does this PR introduce a breaking change or deprecate an API?
If yes, please check if the following requirements are fulfilled
What changes might users need to make in their application due to this PR? (migration steps)
Other information: