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

Rao parameters remove curative stop criterion #1139

Open
wants to merge 2 commits into
base: rao_parameters_forbid_cost_increase_always_true
Choose a base branch
from

Conversation

pjeanmarie
Copy link
Member

@pjeanmarie pjeanmarie commented Sep 18, 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?

What kind of change does this PR introduce?

What is the current behavior?

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

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)

    prev_secure = "preventive-stop-criterion" not in old_obj_fun or old_obj_fun["preventive-stop-criterion"] == "SECURE"
    if prev_secure:
        if "optimize-curative-if-preventive-unsecure" in old_obj_fun:
            new_obj_fun["enforce-curative-security"] = old_obj_fun["optimize-curative-if-preventive-unsecure"]
    else:
        cur_secure = "curative-stop-criterion" in old_obj_fun and old_obj_fun["curative-stop-criterion"] in ("SECURE", "PREVENTIVE_OBJECTIVE_AND_SECURE")
        if cur_secure:
            new_obj_fun["enforce-curative-security"] = True
        else:
            new_obj_fun["enforce-curative-security"] = False
            cur_min = "curative-stop-criterion" in old_obj_fun and old_obj_fun["curative-stop-criterion"] == "MIN_OBJECTIVE"
            if cur_min:
                new_obj_fun["curative-min-obj-improvement"] = 10000.0 + (old_obj_fun["curative-min-obj-improvement"] if "curative-min-obj-improvement" in old_obj_fun else 0.0)

remove ("curative-stop-criterion", "optimize-curative-if-preventive-unsecure") from the objective function parameters

Other information:

@pjeanmarie pjeanmarie linked an issue Sep 18, 2024 that may be closed by this pull request
@pjeanmarie pjeanmarie added breaking-change Changes could break users' code PR : dont-merge-before-other PR mustn't be merged before another (referenced in the description or in the comments) labels Sep 18, 2024
@pjeanmarie pjeanmarie changed the title Rao parameters remove curative stop criterion [WIP] Rao parameters remove curative stop criterion Sep 18, 2024
@pjeanmarie pjeanmarie requested review from phiedw and obrix September 18, 2024 12:44
@pjeanmarie pjeanmarie force-pushed the rao_parameters_forbid_cost_increase_always_true branch from 6ef7c93 to 00713a3 Compare September 23, 2024 08:01
@pjeanmarie pjeanmarie force-pushed the rao_parameters_remove_curative_stop_criterion branch from 22ef4cf to 30c511d Compare September 23, 2024 08:07
@pjeanmarie pjeanmarie force-pushed the rao_parameters_remove_curative_stop_criterion branch from 30c511d to 443782c Compare October 1, 2024 09:33
@pjeanmarie pjeanmarie force-pushed the rao_parameters_forbid_cost_increase_always_true branch from 7fc9c43 to 6607c13 Compare October 2, 2024 12:02
@pjeanmarie pjeanmarie force-pushed the rao_parameters_remove_curative_stop_criterion branch 2 times, most recently from dcc85bc to 7d68245 Compare October 2, 2024 13:59
@pjeanmarie pjeanmarie changed the title [WIP] Rao parameters remove curative stop criterion Rao parameters remove curative stop criterion Oct 2, 2024
@pjeanmarie pjeanmarie force-pushed the rao_parameters_forbid_cost_increase_always_true branch from 6607c13 to b1a5757 Compare October 28, 2024 10:26
@pjeanmarie pjeanmarie force-pushed the rao_parameters_remove_curative_stop_criterion branch from 7d68245 to 9ad0337 Compare October 28, 2024 10:52
@pjeanmarie pjeanmarie force-pushed the rao_parameters_forbid_cost_increase_always_true branch from 82aacb6 to f57fcdb Compare November 15, 2024 09:14
@pjeanmarie pjeanmarie force-pushed the rao_parameters_remove_curative_stop_criterion branch 2 times, most recently from 43b027a to 78c32dc Compare November 15, 2024 11:17
@pjeanmarie pjeanmarie added the PR: waiting-for-review This PR is waiting to be reviewed label Nov 15, 2024
Copy link
Member

@obrix obrix left a comment

Choose a reason for hiding this comment

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

First pass is okay for me, I think some behavior changes were agreed between @pjeanmarie and @phiedw. I think a more precise look at the comment I placed would be good.

@@ -181,8 +181,8 @@ public CompletableFuture<RaoResult> run() {

private boolean shouldStopOptimisationIfPreventiveUnsecure(double preventiveOptimalCost) {
return raoParameters.getObjectiveFunctionParameters().getPreventiveStopCriterion().equals(ObjectiveFunctionParameters.PreventiveStopCriterion.SECURE)
&& preventiveOptimalCost > 0
&& !raoParameters.getObjectiveFunctionParameters().getOptimizeCurativeIfPreventiveUnsecure();
&& preventiveOptimalCost > 0
Copy link
Member

Choose a reason for hiding this comment

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

To check @phiedw

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems correct

// we do not want to run a second preventive that would not be able to fix the situation, to save time
BUSINESS_LOGS.info("First preventive RAO was not able to fix all preventive constraints, second preventive RAO cancelled to save computation time.");
return false;
ObjectiveFunctionParameters.PreventiveStopCriterion preventiveStopCriterion = raoParameters.getObjectiveFunctionParameters().getPreventiveStopCriterion();
Copy link
Member

Choose a reason for hiding this comment

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

To check @phiedw

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this logic is correct

stopCriterion = StopCriterion.MIN_OBJECTIVE;
targetObjectiveValue = 0.0;
targetObjectiveValue = preventiveOptimizedCost - parameters.getObjectiveFunctionParameters().getCurativeMinObjImprovement();
if (parameters.getObjectiveFunctionParameters().getEnforceCurativeSecurity()) {
Copy link
Member

Choose a reason for hiding this comment

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

To check @phiedw

Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks correct

@pjeanmarie pjeanmarie force-pushed the rao_parameters_forbid_cost_increase_always_true branch from f57fcdb to 159c8e2 Compare November 29, 2024 10:17
@pjeanmarie pjeanmarie force-pushed the rao_parameters_remove_curative_stop_criterion branch from 78c32dc to 4371fee Compare November 29, 2024 10:17
…nsecure' and partially replace them by 'enforce curative security'

NB. Curative min objective does no longer exist and is replaced by Curative have the same objective than the Preventive results

Signed-off-by: Pauline Jean-Marie <pauline.jean-marie@artelys.com>
@pjeanmarie pjeanmarie force-pushed the rao_parameters_remove_curative_stop_criterion branch from 4371fee to d726fbb Compare December 9, 2024 10:13
…ps://github.com/powsybl/open-rao into rao_parameters_remove_curative_stop_criterion

Signed-off-by: Philippe Edwards <philippe.edwards@rte-france.com>
Copy link
Collaborator

@phiedw phiedw left a comment

Choose a reason for hiding this comment

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

I think it might make more sense to rename preventive-stop-criterion to stop-criterion now since it affects both preventive and curative steps

@phiedw
Copy link
Collaborator

phiedw commented Dec 12, 2024

I think it might make more sense to rename preventive-stop-criterion to stop-criterion now since it affects both preventive and curative steps

Ah i forgot it's removed in the next pull request anyways so I think it's all good for this pull request! :)

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 PR : dont-merge-before-other PR mustn't be merged before another (referenced in the description or in the comments) PR: waiting-for-review This PR is waiting to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make RaoParameters generic
3 participants