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

Use PowerTransformerEnd.endNumber to sort transformer windings #3234

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

rcourtier
Copy link
Member

@rcourtier rcourtier commented Nov 29, 2024

Please check if the PR fulfills these requirements

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

Does this PR already have an issue describing the problem?

No.

What kind of change does this PR introduce?

Refactoring.

What is the current behavior?

During CGMES conversion of a CIM16 file (CGMES 2.4.15), WindingType class is using Terminal.sequenceNumber to classify and sort the windings as PRIMARY/SECONDARY/TERTIARY. However, sequenceNumber isn't fetched by the SPARQL query and therefore is never used in the WindingType class. As a consequence, WindingType.windingType always returns PRIMARY for CIM16 Propertybags.

This is not an issue because another sort is done in the CGMES import using PowerTransformerEnd.endNumber, giving the correct classifications of windings for CIM16 files, but this additional sorting shouldn't be required and should be done directly in WindingType.

What is the new behavior (if this is a feature change)?

Since the SPARQL queries fetch the PowerTransformerEnd.endNumber and use it to classify and sort the windings as PRIMARY/SECONDARY/TERTIARY, the use of Terminal.sequenceNumber is droped in favor of PowerTransformerEnd.endNumber.

As a consequence, the additional sorting in CGMES import isn't required anymore.

This also gives the iidm WindingType class a clearer purpose which is to support the difference in the modeling of the winding numberings between CIM14 and CIM16.

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

Other information:

This is bad practice to rely on PowerTransformerEnd.Terminal.sequenceNumber. The correct way to classify windings in CIM16 is with the use of PowerTransformerEnd.endNumber.

@rcourtier rcourtier self-assigned this Nov 29, 2024
@rcourtier rcourtier added Breaking Change API is broken and removed Breaking Change API is broken labels Nov 29, 2024
@rcourtier rcourtier force-pushed the use_endnumber_for_powertransformerend_numbering branch 2 times, most recently from 2db89d4 to 2f92b88 Compare December 2, 2024 09:08
@rcourtier rcourtier force-pushed the use_endnumber_for_powertransformerend_numbering branch from 4e1fd3d to f680d6a Compare December 9, 2024 09:23
@rcourtier rcourtier force-pushed the use_endnumber_for_powertransformerend_numbering branch from f680d6a to f6b0c3e Compare January 7, 2025 09:33
@rcourtier rcourtier marked this pull request as ready for review January 7, 2025 09:53
@rcourtier rcourtier requested a review from flo-dup January 7, 2025 13:54
Comment on lines 205 to 211
.forEach(tends -> {
PropertyBags tends1 = new PropertyBags(
tends.getValue().stream()
.sorted(Comparator
.comparing(WindingType::fromTransformerEnd)
.thenComparing(end -> end.asInt(endNumber, -1)))
.collect(Collectors.toList()));
.sorted(Comparator.comparing(WindingType::endNumber))
.toList());
tends.setValue(tends1);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified into

gends.values().forEach(tends -> tends.sort(Comparator.comparing(WindingType::endNumber)));

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.
However, please note that this will be reworked in #3238 and the sorting will be removed from here. Instead, it will be done within the SPARQL query.

@rcourtier rcourtier requested a review from flo-dup January 10, 2025 12:07
@flo-dup flo-dup merged commit 9c18cc5 into main Jan 10, 2025
8 checks passed
@flo-dup flo-dup deleted the use_endnumber_for_powertransformerend_numbering branch January 10, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants