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

Export execution details #1193

Merged
merged 16 commits into from
Dec 5, 2024
Merged

Export execution details #1193

merged 16 commits into from
Dec 5, 2024

Conversation

phiedw
Copy link
Collaborator

@phiedw phiedw commented Nov 6, 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?

Fixes #1137

What kind of change does this PR introduce?

Feature

What is the current behavior?

The RaoResult only exports executed steps (for non failing RAOs)

What is the new behavior (if this is a feature change)?
We now export either the steps executed or a failure reason (as a string). Also surronded the main parts of the RAO to be able to return a FailedRaoResult instead of crashing when an unexpected RuntimeException happens.

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

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

If you use RaoResult.getOptimizationSteps, you will have to replace it with RaoResult.getExecutionDetails

@phiedw phiedw added breaking-change Changes could break users' code feature New feature or request labels Nov 6, 2024
Signed-off-by: Philippe Edwards <[email protected]>
Signed-off-by: Philippe Edwards <[email protected]>
Signed-off-by: Philippe Edwards <[email protected]>
Signed-off-by: Philippe Edwards <[email protected]>
@phiedw
Copy link
Collaborator Author

phiedw commented Nov 8, 2024

Part of this PR can actually make debugging harder because you lose access to the stack trace. Should I add a business error log in the catch to log the error and its stack trace?

@phiedw phiedw added the PR: waiting-for-review This PR is waiting to be reviewed label Nov 13, 2024
@phiedw
Copy link
Collaborator Author

phiedw commented Nov 13, 2024

Part of this PR can actually make debugging harder because you lose access to the stack trace. Should I add a business error log in the catch to log the error and its stack trace?

Added a business error

@@ -66,8 +65,8 @@ public RaoResult deserialize(JsonParser jsonParser, DeserializationContext deser
raoResult.setComputationStatus(deserializeStatus(jsonParser.nextTextValue()));
break;

case OPTIMIZATION_STEPS_EXECUTED:
raoResult.setOptimizationStepsExecuted(deserializeOptimizedStepsExecuted(jsonParser.nextTextValue()));
case OPTIMIZATION_STEPS_EXECUTED, EXECUTION_DETAILS:
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be handled as a version increment, with a checkDeprecatedField ?


if (!(raoResult instanceof FailedRaoResultImpl)) {
OptimizationStepsExecuted optimizationStepsExecuted = raoResult.getOptimizationStepsExecuted();
jsonGenerator.writeStringField(OPTIMIZATION_STEPS_EXECUTED, serializeOptimizedStepsExecuted(optimizationStepsExecuted));
jsonGenerator.writeStringField(EXECUTION_DETAILS, raoResult.getExecutionDetails());
Copy link
Collaborator

Choose a reason for hiding this comment

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

EXECUTION_DETAILS written twice

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm surprised this was not caught by a unit test, I'll add one

assertEquals("The RaoResult object should not be modified outside of its usual routine", exception.getMessage());
exception = assertThrows(OpenRaoException.class, () -> output.setOptimizationStepsExecuted(OptimizationStepsExecuted.FIRST_PREVENTIVE_FELLBACK_TO_INITIAL_SITUATION));
assertEquals("The RaoResult object should not be modified outside of its usual routine", exception.getMessage());
setUp();
Copy link
Collaborator

Choose a reason for hiding this comment

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

already called with @beforeeach

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

indeed!

@Godelaine Godelaine added the PR: waiting-for-correction This PR is waiting to be corrected by its author label Nov 28, 2024
bqth29 and others added 6 commits December 2, 2024 16:29
# Conflicts:
#	data/rao-result/rao-result-api/src/test/java/com/powsybl/openrao/data/raoresult/api/OptimizationStepsExecutedTest.java
#	data/rao-result/rao-result-io/rao-result-json/src/main/java/com/powsybl/openrao/data/raoresult/io/json/RaoResultJsonConstants.java
#	data/rao-result/rao-result-io/rao-result-json/src/main/java/com/powsybl/openrao/data/raoresult/io/json/serializers/RaoResultSerializer.java
#	data/rao-result/rao-result-io/rao-result-json/src/test/java/com/powsybl/openrao/data/raoresult/io/json/RaoResultJsonConstantsTest.java
#	ra-optimisation/search-tree-rao/src/main/java/com/powsybl/openrao/searchtreerao/result/impl/FailedRaoResultImpl.java
#	ra-optimisation/search-tree-rao/src/test/java/com/powsybl/openrao/searchtreerao/result/impl/FailedRaoResultImplTest.java
Signed-off-by: Thomas Bouquet <[email protected]>
… ordering of states in range action results

Signed-off-by: Philippe Edwards <[email protected]>
Signed-off-by: Godelaine de Montmorillon <[email protected]>
@Godelaine Godelaine merged commit 50dfbeb into main Dec 5, 2024
12 checks passed
@Godelaine Godelaine deleted the export_execution_details branch December 5, 2024 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Changes could break users' code feature New feature or request PR: waiting-for-correction This PR is waiting to be corrected by its author PR: waiting-for-review This PR is waiting to be reviewed
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add messages to RaoResult
3 participants