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

TwoWindings and ThreeWindings new Cgmes conversion #1066

Merged
merged 224 commits into from
Jan 14, 2020

Conversation

marqueslanauja
Copy link
Contributor

@marqueslanauja marqueslanauja commented Dec 10, 2019

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)
  • Docs have been added / updated (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

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature

What is the current behavior? (You can also link to an open issue here)
Current Tx Cgmes conversion is done in two steps. In the first step transformer attributes are assigned to all transformers. In the second step the ratioTapChanger and phaseTapChanger Cgmes conversion is done for all tapChangers.

What is the new behavior (if this is a feature change)?
Cgmes conversion of each transformer (Two and Three windings) is concentrated in one step.

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?)
    Both, current and new Cgmes conversion are maintained in the code at the moment and it is possibly to switch between them. Deprecated label has been added to current Cgmes conversion non-private methods.

Other information:
(if any of the questions/checkboxes don't apply, please delete them entirely)

CurrentCgmesConversion is defined in the following four files:

  • TwoWindingsTransformerConversion.java
  • ThreeWindingsTransformerConversion.java
  • RatioTapChangerConversion.java
  • PhaseTapChangerConversion.java.

At the end of the process the four files will be deleted.

NewCgmesConversion is defined in the next files:

  • NewTwoWindingsTransformerConversion.java. New Cgmes conversion for two windings transformers.
  • NewThreeWindingsTransformerConversion.java. New Cgmes conversion for three windings transformers.
  • AbstractTransformerConversion.java. Common methods used in previous classes.
  • TapChangerConversion.java. Class to record Cgmes attributes of a tapChanger (ratio or phase)

After deleting the currentCgmesConversion NewTwoWindingsTransformerConversion and NewThreeWindingsTransformerConversion will be renamed TwoWindingsTransformerConversion and ThreeWindingsTransformerConversion

The NewCgmesConversion of each transformer is divided in four steps:

  • load. Cgmes attributes are collected.
  • interpret. Cgmes data is interpreted according to the configured alternative.
  • convert. Cgmes data is converted to IIDM.
  • set. Transformer IIDM model is set.

Brief description of the changed files:

Context.java ratio and phase tapChanger tables are cached.
Conversion.java. Switch between both Cgmes conversions. Methods to configure the different alternatives supported in the interpretation process.
RegulatingControlMappingForTransformers.java. Extend regulating control to all possible tapChangers in a threeWindingsTransformer.
ConversionTester.java. Temporary modification of the test to compare newCgmesConversion and currentCgmesConversion.
Comparison.java, Differences.java, DifferencesFail.java, DifferencesReport.java. Improve buses comparison to support node breaker models.
CgmesModel.java, CgmesModelTripleStore.java, CIM16.sparql, FakeCgmesModel.java. Add phaseTapChanger tables points
BranchData.java. Add phaseAngleClock.
TwtData.java. Add phaseAngleClock, g and b in all legs and ratedU0.
TwtTestData.java. Adjust g, b in all legs, phaseAngleClock and ratedU0.
ThreeWindingsTransformerImpl.java Extend some methods to all possible tapChangers.
LoadflowValidation and LoadflowResultsCompletion files. Add phaseAngleClock, ratedU0 and g and b in all legs.

marqueslanauja and others added 30 commits October 1, 2019 10:57
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
…(step 1: generators)

Signed-off-by: Luma Zamarreño <zamarrenolm@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>

# Conflicts:
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/RegulatingControlMapping.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/RegulatingControlMappingForGenerators.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/RegulatingControlMappingForTransformers.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/elements/ExternalNetworkInjectionConversion.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/elements/PhaseTapChangerConversion.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/elements/RatioTapChangerConversion.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/elements/SynchronousMachineConversion.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/elements/TwoWindingsTransformerConversion.java
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
… enabled.

Signed-off-by: José Antonio Marqués <marquesja@aia.es>
…rsion_svcRegulatingControl

Signed-off-by: José Antonio Marqués <marquesja@aia.es>

# Conflicts:
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/RegulatingControlMapping.java
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/elements/StaticVarCompensatorConversion.java
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
…rsion_regulatingControlUniformization

Signed-off-by: José Antonio Marqués <marquesja@aia.es>

# Conflicts:
#	cgmes/cgmes-conversion/src/main/java/com/powsybl/cgmes/conversion/RegulatingControlMappingForStaticVarCompensators.java
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
@marqueslanauja
Copy link
Contributor Author

For readibility purposes, I think it would be better to really separate CgmesT3xModel, InterpretedT3xModel and ConvertedT3xModel in three separate classes, rather than making them three subclasses in NewThreeWindingsTransformerConversion. This way, each class can have the methods dealing with them and it will be easier to discriminate particularly between interpretation and pure conversion.

Same remark for CgmesT2xModel, InterpretedT2xModel and ConvertedT2xModel.
Once this is done, it is okay for me to merge!

Done

Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Copy link
Contributor

@miovd miovd left a comment

Choose a reason for hiding this comment

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

Some minor remarks.
Could you also rename TapChanger, TapChangerBuilder, RatioTapChangerBuilder and PhaseTapChangerBuilder respectively as CgmesTapChanger, CgmesTapChangerBuilder, CgmesRatioTapChangerBuilder and CgmesPhaseTapChangerBuilder?

@@ -0,0 +1,54 @@
package com.powsybl.cgmes.conversion.elements.transformers;
Copy link
Contributor

Choose a reason for hiding this comment

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

Add copyright (2020)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I have not change TapChanger to CgmesTapChanger as it is used also in interpreted and converted models and will be confusing.

Comment on lines 43 to 60
static class CgmesEnd {
final double g;
final double b;
final TapChanger ratioTapChanger;
final TapChanger phaseTapChanger;
final double ratedU;
final String terminal;

CgmesEnd(PropertyBag bagEnd, double x, Context context) {
this.g = bagEnd.asDouble(CgmesNames.G, 0);
this.b = bagEnd.asDouble(CgmesNames.B);
this.ratioTapChanger = TapChanger.ratioTapChangerFromEnd(bagEnd, context);
this.phaseTapChanger = TapChanger.phaseTapChangerFromEnd(bagEnd, x, context);
this.ratedU = bagEnd.asDouble(CgmesNames.RATEDU);
this.terminal = bagEnd.getId(CgmesNames.TERMINAL);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think CgmesT2xModel.CgmesEnd and CgmesT3xModel.CgmesEnd could be fused in one unique class CgmesEnd, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to keep them separated since in twoWindingsTransformers to build the phaseTapChanger you need the reactance of the transformer and this reactance is not always the reactante of the CgmesEnd.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay!

@miovd miovd self-requested a review January 13, 2020 09:50
@miovd
Copy link
Contributor

miovd commented Jan 13, 2020

I approved this PR since all the comments I made are minor. Regarding the refactoring of CgmesT2xModel.CgmesEnd and CgmesT3xModel.CgmesEnd, it can be done later in another PR, it was just to have your opinion.

Copy link
Member

@annetill annetill left a comment

Choose a reason for hiding this comment

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

I approve this PR, just a small comment to improve the comment of "conversion.java".

marqueslanauja and others added 2 commits January 13, 2020 17:52
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: RALAMBOTIANA MIORA <miora.ralambotiana@rte-france.com>
@sonarcloud
Copy link

sonarcloud bot commented Jan 14, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 19 Code Smells

80.8% 80.8% Coverage
0.4% 0.4% Duplication

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.

None yet

5 participants