-
Notifications
You must be signed in to change notification settings - Fork 41
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
} | ||
|
||
private static void addTieFlow(Context context, CgmesControlAreas cgmesControlAreas, PropertyBag tf) { |
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 a TieFlowConversion.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
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]>
Quality Gate passedIssues Measures |
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: