-
Notifications
You must be signed in to change notification settings - Fork 49
All iidm objects properties bearers #3669
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
IIDM API and in-memory implementation All the listed interfaces should extend PropertiesHolder. All the listed classes should extend AbstractPropertiesHolder. Signed-off-by: Fabrice Buscaylet <[email protected]>
Signed-off-by: Fabrice Buscaylet <[email protected]>
…use a lot of code changes for just a simple test where it is used Signed-off-by: Fabrice Buscaylet <[email protected]>
Load / ZipLoad Capability Curves ShuntCompensator VoltageLevels Signed-off-by: Fabrice Buscaylet <[email protected]>
Signed-off-by: Fabrice Buscaylet <[email protected]>
Signed-off-by: Fabrice Buscaylet <[email protected]>
olperr1
left a comment
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.
Here are a few comments to begin with.
I still have files to review, but it could allow you to begin taking them into account.
iidm/iidm-api/src/main/java/com/powsybl/iidm/network/util/OverloadImpl.java
Outdated
Show resolved
Hide resolved
iidm/iidm-api/src/main/java/com/powsybl/iidm/network/IdentifiableAdder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/AbstractIdentifiableAdder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/AbstractInjectionAdder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/AbstractLoadingLimitsAdder.java
Outdated
Show resolved
Hide resolved
...i/src/main/java/com/powsybl/security/limitreduction/result/AbstractReducedLoadingLimits.java
Outdated
Show resolved
Hide resolved
iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/NetworkSerDeTest.java
Outdated
Show resolved
Hide resolved
iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/NetworkSerDeTest.java
Outdated
Show resolved
Hide resolved
iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/NetworkSerDeTest.java
Outdated
Show resolved
Hide resolved
iidm/iidm-serde/src/test/java/com/powsybl/iidm/serde/NetworkSerDeTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Fabrice Buscaylet <[email protected]>
Signed-off-by: Fabrice Buscaylet <[email protected]>
Signed-off-by: Fabrice Buscaylet <[email protected]>
Signed-off-by: Fabrice Buscaylet <[email protected]>
iidm/iidm-api/src/main/java/com/powsybl/iidm/network/util/UnsupportedPropertiesHolder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-api/src/main/java/com/powsybl/iidm/network/util/UnsupportedPropertiesHolder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-api/src/main/java/com/powsybl/iidm/network/util/UnsupportedPropertiesHolder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-api/src/main/java/com/powsybl/iidm/network/util/UnsupportedPropertiesHolder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-api/src/main/java/com/powsybl/iidm/network/util/UnsupportedPropertiesHolder.java
Outdated
Show resolved
Hide resolved
iidm/iidm-impl/src/main/java/com/powsybl/iidm/network/impl/VoltageAngleLimitImpl.java
Show resolved
Hide resolved
iidm/iidm-serde/src/main/java/com/powsybl/iidm/serde/AbstractTransformerSerDe.java
Show resolved
Hide resolved
iidm/iidm-serde/src/main/java/com/powsybl/iidm/serde/AbstractTransformerSerDe.java
Show resolved
Hide resolved
| @@ -10,7 +10,7 @@ | |||
| /** | |||
| * @author Geoffroy Jamgotchian {@literal <geoffroy.jamgotchian at rte-france.com>} | |||
| */ | |||
| public interface ExponentialLoadModelAdder { | |||
| public interface ExponentialLoadModelAdder extends PropertiesHolder { | |||
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.
Having the PropertiesHolder adders is convenient for the deserialization, and even at the object creation via the API, but it may generate a lot of work to support for the custom IIDM implementations maintainers.
Since the deserialization relies on the fact that the adders can have properties, all custom IIDM implementations would have to implement properties support on the adders in order to read v1.15 IIDM networks.
To handle inner elements at deserialization, two mechanisms were introduced for Identifiables:
- in the case the identifiable can be created before reading its inner elements:
- the SerDe extends
AbstractSimpleIdentifiableSerDe - the inner elements are read/added to the identifiable after its creation
- the SerDe extends
- in the other case
- the SerDe extends
AbstractComplexIdentifiableSerDe - when reading the inner elements, a task is memorized in a
List<Consumer<T>>, whereTis compatible with the type of the main Identifier to create. Evey task of this list will be later applied on the Identifier once it is created.
- the SerDe extends
These interfaces are only applicable for Identifiers so they could not be used for the new PropertiesHolder objects, but having these 2 mechanisms in mind, it should be possible to have less modifications in the API.
Also note that with the current IIDM implementation, properties are created in the adder only to be copied after in the real object. This have a memory and time overhead.
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.
Maybe we can think of a mechanism on the adder that allows to memorize tasks that will be automatically added once the object is created.
For instance, an interface for some adders that will have a method to add post-additional tasks that will be automatically applied on the object by the add() method.
If this interface/abstract class can be shared, the change for custom IIDM maintainers will be lighter (and it could be reused later for similar purposes).
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.
I agree on the performances overload (time and memory) of storing and setting back properties on adders .
Your idea of a post creation hook that can be called is interresting but i won't have enough time on the subject this week to implement it.
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.
I had the time to complete all the other enhancements above
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.
Thanks @snoopfab!
|
Extensions and TopologyModel are not handled by this PR. |
Signed-off-by: Fabrice Buscaylet <[email protected]>
|



Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
Implements #3518
What kind of change does this PR introduce?
What is the current behavior?
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change or deprecate an API?
If yes, please check if the following requirements are fulfilled
What changes might users need to make in their application due to this PR? (migration steps)
Other information: