powsybl-open-loadflow icon indicating copy to clipboard operation
powsybl-open-loadflow copied to clipboard

Draft : SA : output a result for contingencies with no impact

Open vmouradian opened this issue 1 year ago • 3 comments

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?

feature

What is the current behavior?

When a contingency is converted to lfContingency befor beeing simulated three options are possible :

  • The contingency leads to an isolated slack bus -> contingency skipped and no result is provided
  • The contingency has no impact -> contingency skipped and no result is provided
  • The contingency is ok -> converted to lfContingency and simulated

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

  • The contingency leads to an isolated slack bus -> contingency skipped and no result is provided
  • The contingency has no impact -> return empty result with PostContingencyComputationStatus.NO_IMPACT
  • The contingency is ok -> converted to lfContingency and simulated

This MR is a draft for the moment, the three cases are separated by a new enum to look more clear. But maybe it is a bit overkill.

New unit tests should still be added.

Another option could be to still create the lfContingency if it has no impact and add a hasNoImpact() method to the class (which won't be the same as hasNoImpact() from propagatedContingency, it is a deeper check)

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)

Other information:

vmouradian avatar Feb 21 '24 09:02 vmouradian

@vmouradian I am not sure we can merge as it is right now :

  • I think we can probably avoid creating another enum for status
  • This means maybe we lack ISOLATED_SLACK_BUS in post contingency computation status
  • Which lead me to think we should maybe do things differently here : PostContingencyComputationStatus as its name indicates is a status after computation. Converged, max_iteration_reached and solver failed refers to the load flow evaluation while no_impact and isolated_slack_bus refers to the application of the contingency.

I think we should probably discussed with @annetill to see what is the most generic approach here.

obrix avatar Apr 16 '24 14:04 obrix

@obrix I agree with you on this, at the time I made the PR the goal was just to show that we have 3 separated cases where two were leading to no result. I created the enum to make it more visual but I also think that it is overkill for a final solution

vmouradian avatar Apr 17 '24 13:04 vmouradian