Skip to content
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

Synchronize parameter retrieval methods with PlatformConfig overrides #3066

Closed
antoinebhs opened this issue Jun 18, 2024 · 2 comments · Fixed by #3133 or powsybl/powsybl-network-conversion-server#156

Comments

@antoinebhs
Copy link
Contributor

antoinebhs commented Jun 18, 2024

Describe the current behavior

In the current implementation, we can retrieve a List<Parameter> from the following methods:

Importer.getParameters()
Exporter.getParameters()
LoadFlowProvider.getSpecificParameters()
ShortCircuitAnalysisProvider.getSpecificParameters()

These methods return the default parameters from their respective Provider/Importer/Exporter implementations. However, they do not account for the default parameter overrides specified in the PlatformConfig (in .itools/config.yml).

For example, in open-loadflow, the method returns a hard-coded list of default parameters: https://github.com/powsybl/powsybl-open-loadflow/blob/3404c7fa00058d278f0a78915653cab4e103a5b3/src/main/java/com/powsybl/openloadflow/OpenLoadFlowProvider.java#L312.
The issue is that this list is not synchronized with the parameters actually used in the calculation, which correctly accounts for the PlatformConfig if the default parameters are overridden with open-loadflow-default-parameters: in .itools/config.yml.

Describe the expected behavior

Importer.getParameters()
Exporter.getParameters()
LoadFlowProvider.getSpecificParameters()
ShortCircuitAnalysisProvider.getSpecificParameters()

should retrieve a List<Parameter> that reflects both the default parameters set in their respective Provider/Importer/Exporter implementations and any overrides specified in the PlatformConfig (in .itools/config.yml).

For instance, in open-loadflow, instead of returning a hard-coded list of default parameters, the method should dynamically generate this list, incorporating any overrides defined under open-loadflow-default-parameters: in .itools/config.yml. This ensures that the parameters retrieved are synchronized with the ones specified in the PlatformConfig.

Describe the motivation

In GridSuite, we retrieve the List<Parameter> from providers/importers and only save the parameters that differ from this list. However, the current implementation of these methods is not synchronized with the actual parameters used in the calculation, as it does not account for the PlatformConfig overrides.

Due to this discrepancy, we must manually hard-code the overrides in the code, as there is no straightforward way to retrieve the default parameters with the PlatformConfig overrides applied. This adds complexity and redundancy.

For example, in the LoadFlow parameters, LoadFlowService.java#L81-L97, there is a manual override that would be unnecessary if the methods accurately reflected the PlatformConfig settings. Synchronizing these methods with PlatformConfig would simplify parameter management. We would only need to modify the default parameters in .itools/config.yml with no code addition.

Extra Information

No response

@olperr1
Copy link
Member

olperr1 commented Aug 27, 2024

⚠️ Another better proposal is available in the next comment.

Proposal

Concept

The signature of the getParameters() and getSpecificParameters() methods will not change and their return type will still be List<Parameter>. But instead of returning a list of Parameter objects, they will return a list of objects extending Parameter and having the dynamically defined default value.
The getDefaultValue() method of these objects will return the dynamically defined default value, but the static default value will still be accessible via another getter: getStaticDefaultValue(), in case it is needed (for instance, it can be useful to detect default values that were modified).

Details

A new class DynamicValueParameter extending Parameter will be introduced. It will contain an additional attribute staticDefaultValue.

It will have a single constructor defined as private:
private DynamicValueParameter(Parameter parameter, Object dynamicDefaultValue).

This constructor should:

  • check that dynamicDefaultValue is acceptable regarding parameter's possible values;
  • store dynamicDefaultValue as its defaultValue;
  • store parameter.getDefaultValue() as its staticDefaultValue;
  • store each other parameter's attribute as its own attribute of the same name.

DynamicValueParameter will have 2 public static methods:

  1. List<Parameter> load(Collection<Parameter> parameters, String prefix, ParameterDefaultValueConfig defaultValueConfig)
  2. List<Parameter> load(Collection<Parameter> parameters, ModuleConfig moduleConfig)

They both will return a list of DynamicValueParameter, containing an object for each parameter of the parameters attribute. The dynamic default value will be retrieved:

  • for the 1st method: via defaultValueConfig.getValue(prefix, parameter);
  • for the 2nd method: by calling the moduleConfig.get...Property() method corresponding to the parameter's type.

These methods will be called in the getParameters() and getSpecificParameters() methods with the same arguments as the ones used to read the actual parameter's value.

For instance, UcteImporter.getParameters() will be:

    @Override
    public List<Parameter> getParameters() {
        return DynamicValueParameter.load(PARAMETERS, getFormat(), defaultValueConfig);
    }

instead of:

    @Override
    public List<Parameter> getParameters() {
        return PARAMETERS;
    }

@olperr1
Copy link
Member

olperr1 commented Sep 25, 2024

Updated proposal

Concept

The signature of the getParameters() and getSpecificParameters() methods will not change and their return type will still be List. But instead of returning a list of Parameter objects, they will return a list of objects extending Parameter and having the value defined in configuration.
The getDefaultValue() method of these objects will return the value defined in configuration, but the hard-coded default value will still be accessible via another getter: getBaseDefaultValue(), in case it is needed.

Details

A new class ConfiguredParameter extending Parameter will be introduced. It will contain an additional attribute baseDefaultValue.

It will have a single constructor defined as private:
private ConfiguredParameter(Parameter parameter, Object configuredValue).

This constructor should:

  • store configuredValue as its defaultValue;
  • store parameter.getDefaultValue() as its baseDefaultValue;
  • store each other parameter's attribute as its own attribute of the same name.

ConfiguredParameter will have 2 public static methods:

  1. List<Parameter> load(Collection<Parameter> parameters, String prefix, ParameterDefaultValueConfig defaultValueConfig)
  2. List<Parameter> load(Collection<Parameter> parameters, ModuleConfig moduleConfig)

They both will return a list of Parameter, containing an object for each parameter of the parameters attribute. This object could be either a ConfiguredParameter for a parameter having a defined value in configuration, or a Parameter instance if no value is defined in configuration.
The configured value will be retrieved:

  • for the 1st method: via defaultValueConfig.getValue(prefix, parameter);
  • for the 2nd method: by calling the moduleConfig.get...Property() method corresponding to the parameter's type.

These methods will be called in the getParameters() and getSpecificParameters() methods with the same arguments as the ones used to read the actual parameter's value.

For instance, UcteImporter.getParameters() will be:

    @Override
    public List<Parameter> getParameters() {
        return ConfiguredParameter.load(PARAMETERS, getFormat(), defaultValueConfig);
    }

instead of:

    @Override
    public List<Parameter> getParameters() {
        return PARAMETERS;
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants