Skip to content
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

[WIP] CGMES metadata is now in one extension #2034

Closed
wants to merge 5 commits into from
Closed

Conversation

miovd
Copy link
Contributor

@miovd miovd commented Mar 23, 2022

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

  • The commit message follows our guidelines
  • 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:

  • 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 miovd added CGMES Breaking Change API is broken Deprecated Methods have been deprecated labels Mar 23, 2022
@miovd miovd requested review from zamarrenolm and marcosmc March 23, 2022 15:26
@zamarrenolm
Copy link
Member

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.

@miovd
Copy link
Contributor Author

miovd commented Mar 24, 2022

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 miovd changed the title CGMES metadata is now in one extension [WIP] CGMES metadata is now in one extension Mar 24, 2022
@zamarrenolm
Copy link
Member

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.

@miovd
Copy link
Contributor Author

miovd commented Mar 24, 2022

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?

@zamarrenolm
Copy link
Member

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>

miovd added 2 commits March 25, 2022 14:39
Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
miovd added 3 commits March 25, 2022 15:02
Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 14 Code Smells

91.5% 91.5% Coverage
0.0% 0.0% Duplication

@miovd
Copy link
Contributor Author

miovd commented Mar 28, 2022

@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?

@zamarrenolm
Copy link
Member

@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 ?

@annetill annetill requested review from rcourtier and removed request for marcosmc June 12, 2023 11:52
@annetill annetill requested a review from nemanja-st September 5, 2023 11:43
@nemanja-st
Copy link
Contributor

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.

@zamarrenolm
Copy link
Member

zamarrenolm commented Nov 8, 2023

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.

@nemanja-st
Copy link
Contributor

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.

@zamarrenolm zamarrenolm mentioned this pull request Feb 13, 2024
7 tasks
@annetill
Copy link
Member

Rework through #2898

@annetill annetill closed this Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change API is broken CGMES Deprecated Methods have been deprecated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants