-
Notifications
You must be signed in to change notification settings - Fork 51
add controls on rates for generatorStartUp extension #3716
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
base: main
Are you sure you want to change the base?
Conversation
| if (plannedOutageRate > 1) { | ||
| plannedOutageRate = 1; | ||
| } else if (plannedOutageRate < 0) { | ||
| plannedOutageRate = 0; | ||
| } |
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.
Could you use a small method to avoid code duplication?
For example:
private double readOutageRate(DeserializerContext context, String attributeName) {
double outageRate = context.getReader().readDoubleAttribute(attributeName);
// compatibility
if (outageRate > 1) {
outageRate = 1.0;
} else if (outageRate < 0) {
outageRate = 0.0;
}
return outageRate;
}| } | ||
|
|
||
| public static void checkRate(Validable validable, String validableType, double rate, String fieldName) { | ||
| if (!Double.isNaN(rate) && (rate < 0 || rate > 1)) { |
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.
Could the values be defined as a public static variables somewhere, in order to avoid having the values in multiple places directly in the code?
| @@ -0,0 +1,45 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <iidm:network xmlns:iidm="http://www.powsybl.org/schema/iidm/1_15" xmlns:gs="http://www.powsybl.org/schema/iidm/ext/generator_startup/1_1" id="sim1" caseDate="2026-01-14T10:21:31.745Z" forecastDistance="0" sourceFormat="test" minimumValidationLevel="STEADY_STATE_HYPOTHESIS"> | |||
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.
| <iidm:network xmlns:iidm="http://www.powsybl.org/schema/iidm/1_15" xmlns:gs="http://www.powsybl.org/schema/iidm/ext/generator_startup/1_1" id="sim1" caseDate="2026-01-14T10:21:31.745Z" forecastDistance="0" sourceFormat="test" minimumValidationLevel="STEADY_STATE_HYPOTHESIS"> | |
| <iidm:network xmlns:iidm="http://www.powsybl.org/schema/iidm/1_16" xmlns:gs="http://www.powsybl.org/schema/iidm/ext/generator_startup/1_1" id="sim1" caseDate="2026-01-14T10:21:31.745Z" forecastDistance="0" sourceFormat="test" minimumValidationLevel="STEADY_STATE_HYPOTHESIS"> |
13348a9 to
6c03064
Compare
rolnico
left a comment
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.
commit history + DCO have to be fixed
|
|
||
| public static void checkRate(Validable validable, String validableType, double rate, String fieldName) { | ||
| if (!Double.isNaN(rate) && (rate < MIN_RATE || rate > MAX_RATE)) { | ||
| throw new ValidationException(validable, "Unexpected value for " + fieldName + " of " + validableType + " : " + rate + " is not included in [0, 1]"); |
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.
Use the constants in the message too
| private double readOutageRate(DeserializerContext context, String attributeName) { | ||
| double outageRate = context.getReader().readDoubleAttribute(attributeName); | ||
| // compatibility | ||
| if (outageRate > 1) { |
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.
use the constants
| assertEquals(5.0, generatorStartup.getStartupCost()); | ||
| assertEquals(10.0, generatorStartup.getMarginalCost()); | ||
| // planned outage rate is greater than 1 so it is set to 1 | ||
| assertEquals(1, generatorStartup.getPlannedOutageRate()); |
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.
Compare to the constants
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: Etienne LESOT <[email protected]>
Signed-off-by: EtienneLt <[email protected]>
f416c34 to
0dcf9d2
Compare
Signed-off-by: Etienne LESOT <[email protected]>
|



Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
no
What kind of change does this PR introduce?
feature
What is the current behavior?
for the extension GeneratorStartUp, there is no control on rate fields. But they should be between 0 and 1 to be coherent
What is the new behavior (if this is a feature change)?
add tests
Does this PR introduce a breaking change or deprecate an API?
Other information:
we try to keep compatibility with the SerDe. if in a network there is a value below 0 it is set to 0 and if the value is higher than 1 it is set to 1