Refactor jacobian matrix update #216
Conversation
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
# Conflicts: # src/main/java/com/powsybl/openloadflow/sa/OpenSecurityAnalysis.java # src/main/java/com/powsybl/openloadflow/sensi/DcSensitivityAnalysis.java
| @@ -81,8 +81,11 @@ private void reIndex() { | |||
| for (Equation equation : equations.values()) { | |||
| if (equation.isActive()) { | |||
| NavigableMap<Variable, List<EquationTerm>> equationTermsByVariable = null; | |||
| // check we have at lest one equation term active | |||
| @@ -12,7 +12,6 @@ | |||
| public enum EquationEventType { | |||
| EQUATION_CREATED, | |||
| EQUATION_REMOVED, | |||
| EQUATION_UPDATED, | |||
There was a problem hiding this comment.
You have removed that type because now you want to introduce the equation term granularity ?
src/main/java/com/powsybl/openloadflow/equations/EquationSystem.java
Outdated
Show resolved
Hide resolved
| @@ -81,8 +81,11 @@ private void reIndex() { | |||
| for (Equation equation : equations.values()) { | |||
There was a problem hiding this comment.
As EquationCache is becoming bigger you could maybe put it in a separate file
There was a problem hiding this comment.
I don't know, EquationCache need to access to equations field of EquationSystem, so I would have to pass EquationSystem to EquationCache constructor
| * @author Geoffroy Jamgotchian <geoffroy.jamgotchian at rte-france.com> | ||
| */ | ||
| public enum EquationTermEventType { | ||
| EQUATION_TERM_ADDED, |
There was a problem hiding this comment.
I think we could avoid this third type, as equation terms are always added at equation creation.
There was a problem hiding this comment.
In the actual code, yes but the Equation API allows to add new terms after creation so maybe we should still keep it.
There was a problem hiding this comment.
Ok. The equation API is a bit strange in fact, as the equation has a temporary state when not containing any terms (or not containing all terms), and that temporary state doesn't really make sense. But ok to keep it that way.
src/main/java/com/powsybl/openloadflow/equations/JacobianMatrix.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
Signed-off-by: Geoffroy Jamgotchian <geoffroy.jamgotchian@rte-france.com>
|
Kudos, SonarCloud Quality Gate passed! |
Please check if the PR fulfills these requirements (please use
'[x]'to check the checkboxes, or submit the PR and then click the checkboxes)Does this PR already have an issue describing the problem ? If so, link to this issue using
'#XXX'and skip the restNo
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Refactoring
What is the new behavior (if this is a feature change)?
The equation system to jacobian matrix has been refactor to better encapsulate in
JacobianMatrixthe update logic.EquationSystemnotification has been improve to give more informations toJacobianMatrix.This refactoring will allow later to improve performance.
Does this PR introduce a breaking change or deprecate an API? If yes, check the following:
Other information:
(if any of the questions/checkboxes don't apply, please delete them entirely)