Simplify loadflow call

[geofjamg]

• Do you want to request a feature or report a bug? Feature

• What is the current behavior? Here is a typical code snippet to run a load flow

Network network = ...
ComponentDefaultConfig defaultConfig = ... 
LoadFlowFactory = ...
LoadFlow loadFlow = loadFlowFactory.create(network, ....);
LoadFlowResult result = loadFlow.run(network.getVariantManager().getWorkingVariant(), LoadFlowParameters.load()).join();

It is cubersome and most of the time we want to run the load flow on working variant and use default parameters.

• If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem N/A

• What is the expected behavior? I propose to Improve loadflow interface adding new convenient methods like this:

public interface LoadFlow extends Versionable {

    static LoadFlow create(Network network, ComputationManager computationManager) {
         ...
    } 

    static LoadFlow create(Network network) {
         return create(network, LocalComputationManager.getDefault());
    } 

    static LoadFlowResult run(Network network, ComputationManager computationManager, LoadFlowParameters parameters) {
        ...
    }

    static LoadFlowResult run(Network network, LoadFlowParameters parameters) {
        return run(network, LocalComputationManager.getDefault(), parameters);
    }

    static LoadFlowResult run(Network network) {
        return run(network, LoadFlowParameters.load());
    }

    CompletableFuture<LoadFlowResult> run(String workingStateId, LoadFlowParameters parameters);

    // run a loaflow on working variant 
    CompletableFuture<LoadFlowResult> run(LoadFlowParameters parameters);

    // run a loadflow on working variant and loading config from platform config utility class 
    CompletableFuture<LoadFlowResult> run();
}

So we would be able to refactor previous code like this:

Network network = ...
LoadFlowResult result = LoadFlow.run(network);

And we could provide an abstract class to help implementation:

public abstract class AbstractLoadFlow implement LoadFlow {

   protected abstract Network getNetwork();

    public CompletableFuture<LoadFlowResult> run(LoadFlowParameters parameters) {
        return run(getNetwork().getVariantManager().getWorkingVariant(), parameters);
    }

    public CompletableFuture<LoadFlowResult> run() {
         return run(LoadFlowParameters.load());
    }
}

• What is the motivation / use case for changing the behavior?

The aim is to simplify loadflow API.

• Please tell us about your environment:

  • PowSyBl Version: ...
  • OS Version: 2.5.0-SNAPSHOT

• Other information (e.g. detailed explanation, stacktraces, related issues, suggestions how to fix, links for us to have context, eg. stackoverflow, spectrum, etc)

(if a question doesn't apply, you can delete it)

[sylvlecl]

I think here we must make a clear choice between what is the SPI (what interface should implement a loadflow provider), and what is the user API.

Maybe we should have different classes for those 2 goals. I think the current Loadflow class is more the SPI, while we could have layers above it to make the user life easier.

[geofjamg]

I think here we must make a clear choice between what is the SPI (what interface should implement a loadflow provider), and what is the user API.

Maybe we should have different classes for those 2 goals. I think the current Loadflow class is more the SPI, while we could have layers above it to make the user life easier.

So we would have LoadFlow class for API and implementation would extends another one like LoadFLowProvider ?

[geofjamg]

I think here we must make a clear choice between what is the SPI (what interface should implement a loadflow provider), and what is the user API. Maybe we should have different classes for those 2 goals. I think the current Loadflow class is more the SPI, while we could have layers above it to make the user life easier.

So we would have LoadFlow class for API and implementation would extends another one like LoadFLowProvider ?

But looking at existing design LoadFlow is the API and LoadFlowFactory is kind of SPI?

[sylvlecl]

So we would have LoadFlow class for API and implementation would extends another one like LoadFLowProvider ?

For example !

Basically a loadflow provider, I mean someone who whants to plug his own tool for example, only needs to implement one method. He should not have to bother with implementing convenience method which are common to all implementations. For this purpose, you propose to have an Abstract class which implements those convenience methods.

Another possibility is to leave the SPI minimalist (1 method), and to implement convenience methods elsewhere. I think this has some advantages:

• not enforcing the provider to extend the abstract class
• guaranteeing the behaviour of the convenience methods (they cannot be re-implemented in load flow implementations)
• maybe make the SPI more stable

Then the API actually used by users could be for example a class wrapping the SPI class (LoadFlowProvider), or static methods in a LoadFlows. Indeed it would seem better to keep LoadFlow word for the user API.

Finally, it could be a good time to review the loadflow SPI, which currently consists in 2 classes : LoadFlow and LoadFlowFactory. We can question why some arguments are provided through the factory while others are provided through the run method.

[geofjamg]

So we would have LoadFlow class for API and implementation would extends another one like LoadFLowProvider ?

For example !

Basically a loadflow provider, I mean someone who whants to plug his own tool for example, only needs to implement one method. He should not have to bother with implementing convenience method which are common to all implementations. For this purpose, you propose to have an Abstract class which implements those convenience methods.

Another possibility is to leave the SPI minimalist (1 method), and to implement convenience methods elsewhere. I think this has some advantages:

* not enforcing the provider to extend the abstract class

* guaranteeing the behaviour of the convenience methods (they cannot be re-implemented in load flow implementations)

* maybe make the SPI more stable

Then the API actually used by users could be for example a class wrapping the SPI class (LoadFlowProvider), or static methods in a LoadFlows. Indeed it would seem better to keep LoadFlow word for the user API.

Finally, it could be a good time to review the loadflow SPI, which currently consists in 2 classes : LoadFlow and LoadFlowFactory. We can question why some arguments are provided through the factory while others are provided through the run method.

Agree with your comment. I update the issue description to include this and also implementation loading which actually need external config but could rely on service loader to be more automatic.

[geofjamg]

This issue is not to place to write all the code so as a poc I tried to init a refactoring on branch loadflow_api_v2. It does not compile, it is just a poc for discussion. Feedback and also modification welcome!

[geofjamg]

This issue is not to place to write all the code so as a poc I tried to init a refactoring on branch loadflow_api_v2. It does not compile, it is just a poc for discussion. Feedback and also modification welcome!

@sylvlecl @mathbagu Do you think it is the correct way to refactor the loadflow API ?

[mathbagu]

From my point of view, I think some parameters should not be passed to the constructor but to the run method. I know it changes the paradigm but I think a LoadFlow instance should not depend on. We should be able to run loadflow on several networks without creating several LoadFlow instance. I think we have to think more what are LoadFlow attributes (parameters? Implementation?) and what are run/runAsync parameters.

LoadFlow lf = ...
lf.run(network1);
lf.run(network1, variantId);
lf.run(network1, variantId, parameters);
lf.run(network2, parameters);

We could imagine you can pass parameters to the constructor (default parameters) and provide a method to change them for only one call.

I think, we have to introduce a new static method that creates LoadFlow from the implementation name (as we discussed):

LoadFlowResult r1 = LoadFlow.with(“impl“).run(network, variantId);
LoadFlowResult r2 = LoadFlow.with(“impl“, parameters).run(network, variantId);

[geofjamg]

The problem if you had network as a run parameter is that it is not possible to cache structural data of the network common to all runs which could allow in some impl a huge perfirmance improvement

[geofjamg]

The problem if you had network as a run parameter is that it is not possible to cache structural data of the network common to all runs which could allow in some impl a huge perfirmance improvement

A deeper explanation: in most of the cases when you want to run a loadlfow multiple times, it is on multiple variant of the same network and in that case it is very interesting for performance to allow caching some conversion of the structure. If you model you loadflow API totally stateless, you prevent by design any optimization/caching of the structure.

[sylvlecl]

On the other side, 2 calls to the run method with exactlty the same arguments will return different results if you have modified the network in between, which can seem weird : the method does not say anymore what has an impact on the result. Maybe caching could be performed when the run method is called in that case ?

Whatever we choose, your argument holds also for the "provider" side I think.

[mathbagu]

The implementation has to observe the network and invalidate the cache when needed. I think this design could be dangerous/error prone (really easy to forget to handle an event)

[geofjamg]

Ok, I just updated to implementation to pass network in run argument. Another question: there is module like action simulator or loadflow validation that use platform config to configure which loadflow implementation to use (so it does not rely on default implementation). In actual code it is an implementation of LoadFlowFactory that is configured. Agree to replace by the name of the load flow provider to use?