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

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vmouradian
Copy link
Member

@vmouradian vmouradian commented Feb 21, 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?

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:

@obrix obrix force-pushed the feature/sa-result-no-impact branch from b4012a4 to 17be2a4 Compare April 16, 2024 13:59
Copy link

sonarcloud bot commented Apr 16, 2024

@obrix
Copy link
Member

obrix commented Apr 16, 2024

@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.

@vmouradian
Copy link
Member Author

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants