powsybl-core icon indicating copy to clipboard operation
powsybl-core copied to clipboard

[WIP] CGMES metadata is now in one extension

Open miovd opened this issue 2 years ago • 8 comments

Please check if the PR fulfills these requirements (please use '[x]' to check the checkboxes, or submit the PR and then click the checkboxes)

  • [x] The commit message follows our guidelines
  • [x] Tests for the changes have been added (for bug fixes / features)

Does this PR already have an issue describing the problem ? If so, link to this issue using '#XXX' and skip the rest No

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) Feature/Bug fix: handle metadata correctly

Does this PR introduce a breaking change or deprecate an API? If yes, check the following:

  • [x] The Breaking Change or Deprecated label has been added
  • [ ] The migration guide has been updated in the github wiki (What changes might users need to make in their application due to this PR?)

miovd avatar Mar 23 '22 15:03 miovd

Your proposal is great!

Some global comments, let me know what you think:

Maybe we could unify your proposed CgmesMetadata.Model and the CgmesExportContext.ModelDescription. It don't like to have this class duplicated. The export context could use directly the class defined in the extension. Do not need to copy attribute by attribute, just access the extension data when needed.

Instead of CgmesMetadata, I prefer the name CgmesModelDescriptions. The term metadata is too generic for me, we are storing the formal model descriptions defined in the standard.

Instead of explicit methods getEq(), getTp(), getShh(), getSv() in CgmesMetada, I would like to have something like get(CgmesSubset). There are more subsets (DL: Diagram Layout, DY: Dynamic data, GL: GeographicalLocation ...) that could be added later. I think it is ok to enforce the use of this enum in all cgmes-related modules.

zamarrenolm avatar Mar 24 '22 08:03 zamarrenolm

Your proposal is great!

Some global comments, let me know what you think:

Maybe we could unify your proposed CgmesMetadata.Model and the CgmesExportContext.ModelDescription. It don't like to have this class duplicated. The export context could use directly the class defined in the extension. Do not need to copy attribute by attribute, just access the extension data when needed.

Instead of CgmesMetadata, I prefer the name CgmesModelDescriptions. The term metadata is too generic for me, we are storing the formal model descriptions defined in the standard.

Instead of explicit methods getEq(), getTp(), getShh(), getSv() in CgmesMetada, I would like to have something like get(CgmesSubset). There are more subsets (DL: Diagram Layout, DY: Dynamic data, GL: GeographicalLocation ...) that could be added later. I think it is ok to enforce the use of this enum in all cgmes-related modules.

Hi, thank you for your feedback! I agree with you, I will rework this PR following your remarks.

miovd avatar Mar 24 '22 08:03 miovd

Also, I think this is the perfect point to finally implement the list of profiles inside a Model, instead of a single profile like we have now.

zamarrenolm avatar Mar 24 '22 09:03 zamarrenolm

Also, I think this is the perfect point to finally implement the list of profiles inside a Model, instead of a single profile like we have now.

What do you mean? Instead of having a list of Model, CgmesModelDescriptions would have only one Model with several profiles? Isn't it more confusing?

miovd avatar Mar 24 '22 13:03 miovd

Also, I think this is the perfect point to finally implement the list of profiles inside a Model, instead of a single profile like we have now.

What do you mean? Instead of having a list of Model, CgmesModelDescriptions would have only one Model with several profiles? Isn't it more confusing?

CgmesModelDescriptions should contain a list of Models (one for each Subset: EQ, SSH, SV, TP, ...). Each Model has its description, version, authority, dependencies and profiles. For example an EQ model can have the profiles: EquipmentCore, EquipmentOperation, EquipmentShortCircuit. Until now, we stored just one reference to a profile, not a list of possible profiles.

See the following header from the Mini Grid test configuration:

  <md:FullModel rdf:about="urn:uuid:239ecbd2-9a39-11e0-aa80-0800200c9a66">
    <md:Model.scenarioTime>2030-01-02T09:00:00</md:Model.scenarioTime>
    <md:Model.created>2015-02-05T12:20:50.830</md:Model.created>
    <md:Model.description>CGMES Conformity Assessment: Mini Grid Base Case Test Configuration. The model is owned by ENTSO-E and is provided by ENTSO-E "as it is". To the fullest extent permitted by law, ENTSO-E shall not be liable for any damages of any kind arising out of the use of the model (including any of its subsequent modifications). ENTSO-E neither warrants, nor represents that the use of the model will not infringe the rights of third parties. Any use of the model shall include a reference to ENTSO-E. ENTSO-E web site is the only official source of information related to the model.</md:Model.description>
    <md:Model.version>4</md:Model.version>
    <md:Model.profile>http://entsoe.eu/CIM/EquipmentCore/3/1</md:Model.profile>
    <md:Model.profile>http://entsoe.eu/CIM/EquipmentOperation/3/1</md:Model.profile>
    <md:Model.profile>http://entsoe.eu/CIM/EquipmentShortCircuit/3/1</md:Model.profile>
    <md:Model.modelingAuthoritySet>http://A1.de/Planning/ENTSOE/2</md:Model.modelingAuthoritySet>
    <md:Model.Supersedes rdf:resource="urn:uuid:2399cbd2-9a39-11e0-aa80-0800200c9a66_EU" />
    <md:Model.DependentOn rdf:resource="urn:uuid:2399cbd0-9a39-11e0-aa80-0800200c9a66" />
  </md:FullModel>

zamarrenolm avatar Mar 24 '22 15:03 zamarrenolm

@zamarrenolm I've rethink a bit the design of the extension but there are some points I would like to have your opinion on:

  • I would like to maintain two different objects for model in the export context and in the extension for several reasons:
    • they are not used the same: the extension should be immutable, its purpose is to only be set once during CGMES import. When we export a file to CGMES from scratch, it is useless to create this extension... on the contrary, the object in the context is made to be modified by the user before the export. Those objects are similar but not sure if they can be mutualized.
    • it would mean that the model extensions implementation would be mandatory for export which I'm not sure is a good idea
  • I kept only one profile by model in CGMES export: indeed, I think this is better to add the profiles if we decide to export the data associated for this profile (for example short-circuit, etc) rather than adding them in any case if we have them in the configuration... Since we are not exporting data from these profiles yet, I think we could let things as they are now and cross that bridge when we come to it.

What do you think?

miovd avatar Mar 28 '22 15:03 miovd

@zamarrenolm I've rethink a bit the design of the extension but there are some points I would like to have your opinion on:

  • I would like to maintain two different objects for model in the export context and in the extension for several reasons:

    • they are not used the same: the extension should be immutable, its purpose is to only be set once during CGMES import. When we export a file to CGMES from scratch, it is useless to create this extension... on the contrary, the object in the context is made to be modified by the user before the export. Those objects are similar but not sure if they can be mutualized.
    • it would mean that the model extensions implementation would be mandatory for export which I'm not sure is a good idea
  • I kept only one profile by model in CGMES export: indeed, I think this is better to add the profiles if we decide to export the data associated for this profile (for example short-circuit, etc) rather than adding them in any case if we have them in the configuration... Since we are not exporting data from these profiles yet, I think we could let things as they are now and cross that bridge when we come to it.

What do you think?

Agree with your two proposals. Let's go step by step.

About multiple profiles in one model: even if we keep only one, we could maybe prepare the list so later we do not have to change again the xsd and serialization of the extension ?

zamarrenolm avatar Mar 29 '22 10:03 zamarrenolm

Hello @zamarrenolm ,

Regarding your previous discussions regarding profile export, indeed profile which is not used shouldn't be exported. For example, if we export EquipmentOperation without using all the mandatory attributes, Terminal.ConnectivityNode cardinality check would be triggered.

I had one question regarding the implementation: Do you plan to use Model.Supersedes in SSH export? Currently, this is in use in Entsoe process when RCC creates CGM, new SSH has Model.Supersedes reference to the original SSH used in merging.

nemanja-st avatar Nov 03 '23 16:11 nemanja-st

Hello @zamarrenolm ,

Regarding your previous discussions regarding profile export, indeed profile which is not used shouldn't be exported. For example, if we export EquipmentOperation without using all the mandatory attributes, Terminal.ConnectivityNode cardinality check would be triggered.

I had one question regarding the implementation: Do you plan to use Model.Supersedes in SSH export? Currently, this is in use in Entsoe process when RCC creates CGM, new SSH has Model.Supersedes reference to the original SSH used in merging.

About only writing the reference to operation profile if needed: we are trying to do it. We decide if we have to write connectivity nodes based on the the target CGMES version and the IIDM topology kind. If export is CIM 100 (CGMES 3) we always write connectivity nodes. If the export is CIM 16 (CGMES 2.4) we export connectivity nodes only if there is an IIDM voltage level with topology kind node/breaker. This check is done in CgmesExportContext::writeConnectivityNodes. When we write the model description for the output EQ, we include the profile EquipmentOperation only if we are working with CIM < 100 and we have determined that we are working with node/breaker export. See ModelDescriptionEq::write. In CGMES 3 (CIM 100) we don't need to include the Operation profile to be able to refer to connectivity nodes. As far as we have been able to read in the latest UML model, this profile in CGMES 3 refers to "Control" and "Measurements".

About Model.Supersedes: yes, we are already writing it in the exported SSH, when we come from a CGMES case. See SteadyStateHypothesisExport::write, that calls a model write method with information created during import.

These two features are present already in the main branch, they are not specific of this PR.

zamarrenolm avatar Nov 08 '23 09:11 zamarrenolm

About only writing the reference to operation profile if needed: we are trying to do it. We decide if we have to write connectivity nodes based on the the target CGMES version and the IIDM topology kind. If export is CIM 100 (CGMES 3) we always write connectivity nodes. If the export is CIM 16 (CGMES 2.4) we export connectivity nodes only if there is an IIDM voltage level with topology kind node/breaker. This check is done in CgmesExportContext::writeConnectivityNodes. When we write the model description for the output EQ, we include the profile EquipmentOperation only if we are working with CIM < 100 and we have determined that we are working with node/breaker export. See ModelDescriptionEq::write. In CGMES 3 (CIM 100) we don't need to include the Operation profile to be able to refer to connectivity nodes. As far as we have been able to read in the latest UML model, this profile in CGMES 3 refers to "Control" and "Measurements".

About Model.Supersedes: yes, we are already writing it in the exported SSH, when we come from a CGMES case. See SteadyStateHypothesisExport::write, that calls a model write method with information created during import.

These two features are present already in the main branch, they are not specific of this PR.

Thanks for the info, it's clear to me now.

nemanja-st avatar Nov 09 '23 09:11 nemanja-st

Rework through #2898

annetill avatar Feb 19 '24 07:02 annetill