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

add limit to pst range for sensi changes #758

Merged
merged 23 commits into from
Jun 5, 2023

Conversation

MartinBelthle
Copy link
Collaborator

@MartinBelthle MartinBelthle commented May 4, 2023

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Feature

What is the current behavior? (You can also link to an open issue here)

The Rao currently assumes that the impact of a PST on a CNEC is linear. But in AC mode, this approximation is not true.
Therefore, the MIP can choose a solution which is in fact worse than the previous iteration. Currently, when this happens the MIP stops and keeps the best iteration (the last one).

What is the new behavior?

This PR allows the MIP to continue even though it found a worse solution. It keeps in mind the best solution and continue until it reaches max_iteration.

Moreover, we diminish the pst relative variation at each iteration to avoid the MIP to behave as seen on the left picture below.

parabole

The factor 2/3 has been chosen to allow, if needed, the PST to come back to its initial tap. Here is an example of the new behavior:
The PST has 32 taps (-16 to +16). In initial state, its tap position is -16. After the 1st iteration, its tap position is +16 and the MIP has worsen the result. It will run 2nd iteration but for this iteration the PST can only go from tap -5.3 (tap 16 - 32 taps * 2/3) to tap +16. And for the next iterations, its admissible range will shrink to : previous iteration tap +- 32 taps * (2/3)^current iteration number.

Comment on lines 63 to 65
public boolean getCapPstVariation() {
return capPstVariation;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you really need a public getter?

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 use it in 2 different test packages, that is why i made it public. But i do not understand why is this an issue as there are other public getters in the same class.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we usually avoid exposing methods that are not needed in the main part of the code. this makes the design of the classes cleaner.
If you need to test the value, consider another way, or at least making the getter private package. If it's not possible, then you can keep the getter, it's not a big issue.

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 use this method in Leaf.java which doesn't belong to the same package so i left it public (as getStopCriterion, getLeavesInParallel etc. that are used in SearchTree.java)

@pet-mit pet-mit added the PR: waiting-for-correction This PR is waiting to be corrected by its author label May 26, 2023
@MartinBelthle MartinBelthle requested a review from pet-mit May 30, 2023 13:45
@MartinBelthle MartinBelthle requested a review from pet-mit June 2, 2023 16:25
@pet-mit pet-mit merged commit 51f661f into master Jun 5, 2023
@pet-mit pet-mit deleted the add-limit-to-pst-range-for-sensi-changes branch June 5, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: waiting-for-correction This PR is waiting to be corrected by its author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants