-
Notifications
You must be signed in to change notification settings - Fork 43
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 import: fix HVDC active power set point conversion #2012
Conversation
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
…l/powsybl-core into fix-cgmes-conversion-hdvc
@@ -101,19 +101,19 @@ private static double getMaxP(double pAC1, double pAC2, HvdcLine.ConvertersMode | |||
return DEFAULT_MAXP_FACTOR * Math.abs(pAC1); | |||
} | |||
|
|||
private static double getPDc(double pAC1, double pAC2, double poleLossP1, double poleLossP2, | |||
HvdcLine.ConvertersMode mode) { | |||
private static double getActivePowerSetpoint(double pAC1, double pAC2, double poleLossP1, double poleLossP2, |
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.
Maybe add a comment saying that the active power set point of an HVDC should be interpreted on AC side of the rectifier converter
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
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 reviewed all the code and it is ok. Another way to avoid the duplicated code is to add a new method getPdc
in the HvdcLine
interface. The code of the method will be:
public double getPdc() {
if (getConvertersMode() == ConvertersMode.SIDE_1_RECTIFIER_SIDE_2_INVERTER) {
return getActivePowerSetpoint() * (1 - converterStation1.getLossFactor() / 100);
} else {
return getActivePowerSetpoint() * (1 - converterStation2.getLossFactor() / 100);
}
}
@@ -441,9 +441,10 @@ private static void writeConverters(Network network, String cimNamespace, XMLStr | |||
for (HvdcConverterStation<?> converterStation : network.getHvdcConverterStations()) { | |||
double poleLoss; | |||
if (CgmesExportUtil.isConverterStationRectifier(converterStation)) { | |||
poleLoss = converterStation.getLossFactor() * converterStation.getHvdcLine().getActivePowerSetpoint() / (100 - converterStation.getLossFactor()); |
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 think that we may have an issue here... why we don't rely on converter station terminals to get p?
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.
We agree. We should use the terminal P to get poleLoss as a first option and only when terminal P is no available use the current code.
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.
Can you please make the fix?
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.
Yes, I am doing it.
if (hvdcLine != null) { | ||
return hvdcLine.getConverterStation1() == this ? hvdcLine.getConverterStation2() : hvdcLine.getConverterStation1(); | ||
} else { | ||
throw new UnsupportedOperationException(); |
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 don't think we should throw an exception, maybe make the method return an optional that can be empty:
public Optional<HvdcConverterStation> getOtherConverterStation() {
if (hvdcLine != null) {
return hvdcLine.getConverterStation1() == this ? Optional.of(hvdcLine.getConverterStation2()) : Optional.of(hvdcLine.getConverterStation1());
}
return Optional.empty();
}
HvdcLine hvdcLine = getHvdcLine(); | ||
if (hvdcLine != null) { | ||
return hvdcLine.getConverterStation1() == this ? hvdcLine.getConverterStation2() : hvdcLine.getConverterStation1(); | ||
} else { | ||
throw new UnsupportedOperationException(); | ||
} | ||
} |
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.
Only do: return getIndex().getHvdcConverterStation(getDelegate().getHvdcConverterStation());
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: VEDELAGO MIORA <miora.ralambotiana@rte-france.com>
Signed-off-by: José Antonio Marqués <marquesja@aia.es>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
Signed-off-by: Anne Tilloy <anne.tilloy@rte-france.com>
SonarCloud Quality Gate failed. |
Signed-off-by: Anne Tilloy anne.tilloy@rte-france.com
Please check if the PR fulfills these requirements (please use
'[x]'
to check the checkboxes, or submit the PR and then click the checkboxes)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'
and skip the restWhat kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change or deprecate an API? If yes, check the following:
Other information:
(if any of the questions/checkboxes don't apply, please delete them entirely)