-
Notifications
You must be signed in to change notification settings - Fork 49
Security analysis detailed result handler #3625
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: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
Signed-off-by: Bertrand Rix <[email protected]>
|
|
In the proposed approach a new parameter for the security analysis is available : a list of SecurityAnalysisResultHandlers. Theses handlers will be notified in the security analysis implementation when data is available. A basic example implementation is provided, InMemoryResultHandler, that just store the data in memory as they come. The user can provide its own implementation. The contingency and operator strategy is provided in the API to let the user deduce the context of the data provided (see in memory implementation). What is missing here to a least cover our use case : a GeneratorResult, providing target and real flow for each state. But to add this the StateMonitor API should probably be updated to be able to monitor generators. Maybe we could add some more notification API in the handler interface such as "base case done", "contingency done", "operator strategy done". It could be handy when doing large security analysis and some progression notification system is available in the user architecture. |
|
|
||
| void writeBusResult(Contingency contingency, OperatorStrategy operatorStrategy, BusResult busResult); | ||
|
|
||
| } |
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 align with the suggestion in #3626 (that extends the sensitivity writer for multi-component, better failure diagnosis and interrupt) you could add
// numCC and numCs are used for contingencies that have equipments in multiple components
void writeContingencyStatus contingency(int contingencyIndex, LoadFlowResult.ComponentResult.Status, String statusText, int numCC, int numCs);
void writeSynchronousComponentStatus(int numCC, int numCS, LoadFlowResult.ComponentResult.Status, String statusText,);
// Called at the end of the computation if the computation has not been interrupted
void computationComplete();
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 turn this in to a way to get result in streaming, you could also add methods for listening to violations.
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.
@obrix yes, here you only streamed only a part of the SecurityAnalysisResult API. We should fully convert it even for violations.
| /** | ||
| * Sets the list of handlers referenced in {@link SecurityAnalysisResultHandler} | ||
| */ | ||
| public T setResultHandlers(List<SecurityAnalysisResultHandler> handlers) { |
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.
Why a list and not only one ?
If there was only one, we coud mimic the sensi API: no result handler provided means you return an in memory result.
Handler provided means you return void and send the data to the writer.
This view implies that the writer supports also the violation results, not only the monitored objects.
Then if it is useful to have serveral handlers, we would provided the implementation of a SecurityAnalysisResultMultiplexer that would dispatch the calls to a list of handlers.
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 I agree, if some kind of multiplexing is needed the user has everything available to do it himself anyway. So ok for a single handler API.
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.
An handler provided would mean we do not fill the result objects then ? NetworkResult in PreContingencyResult / PostContingencyResult would remain empty ?
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 is the Sensitivity API. We could use the same idea for security + writer
No writer -> returns a Result. Internally it creates a writer that creates the result object. The provider only knows the writer.
public SensitivityAnalysisResult run(Network network,
String workingVariantId,
List factors,
SensitivityAnalysisRunParameters runParameters)
A writer -> returns void. The user code handles that data as needed, without memory overhead, for example if the result is directly streamed into a database or a python dataframe.
public void run(Network network,
String workingVariantId,
SensitivityFactorReader factorReader,
SensitivityResultWriter resultWriter,
SensitivityAnalysisRunParameters runParameters) {
runAsync(network, workingVariantId, factorReader, resultWriter, runParameters).join();
}


Please check if the PR fulfills these requirements
Does this PR already have an issue describing the problem?
What kind of change does this PR introduce?
A proposal for a detailed result handler interface in the security analysis API
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: