-
Notifications
You must be signed in to change notification settings - Fork 51
Optimize getFictitiousP0() and getFictitiousQ0() to compute over every nodes only when needed #3754
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
| if (Double.isNaN(p0)) { | ||
| throw new ValidationException(voltageLevel, "undefined value cannot be set as fictitious p0"); | ||
| } | ||
| hasFictitiousP0.setTrue(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only true for current variant (and if p0 != 0). And you need to handle the setFalse also, if we switch back the value to 0 (and all others are).
|
|
||
| @Override | ||
| public boolean hasFictitiousP0() { | ||
| return hasFictitiousP0.get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to avoid this parameter we could check that fictitiousP0ByNode is empty, and if not sum the (abs?) values of current variant for each node
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this would be cleaner and without any parameter
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, currently, it is considered that any non zero value in any node of the current variant leads to hasFictitiousP0=true. I don't know if there would be the need to be more accurate (sum of the nodes ?)
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/NodeBreakerTopologyModel.java
Outdated
Show resolved
Hide resolved
iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/NodeBreakerTopologyModel.java
Outdated
Show resolved
Hide resolved
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/CalculatedBusImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
| if (fictitiousP0ByNode.isEmpty()) { | ||
| return false; | ||
| } | ||
| int variantIndex = getNetwork().getVariantIndex(); | ||
| for (int node : fictitiousP0ByNode.keys()) { | ||
| TDoubleArrayList p0ByVariant = fictitiousP0ByNode.get(node); | ||
| if (p0ByVariant != null && p0ByVariant.get(variantIndex) != 0.0) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can avoid duplication by creating a small private method with the map (fictitiousP0ByNode or fictitiousQ0ByNode) as argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
Signed-off-by: PRABAKARAN Sylvestre <[email protected]>
|


Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
No
What kind of change does this PR introduce?
Performance issue is observed when using
getFictitiousP0()andgetFictitiousQ0()even when there are no fictitious injection. This PR optimizes this behavior.What is the current behavior?
getFictitiousP0()andgetFictitiousQ0()always search all nodes of the bus to compute fictitious injection.What is the new behavior (if this is a feature change)?
The search is done only if there is at least one non-zero fictitious injection in the voltage level.
Does this PR introduce a breaking change or deprecate an API?