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

Retrieve specific parameters overrides from PlatformConfig #1093

Merged
merged 2 commits into from
Oct 11, 2024

Conversation

antoinebhs
Copy link
Contributor

@antoinebhs antoinebhs commented Sep 27, 2024

Requires next powsybl upgrade

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)

Does this PR already have an issue describing the problem?
powsybl/powsybl-core#3066

@antoinebhs antoinebhs force-pushed the specific-parameters-from-config branch from 498ea97 to c20b8cc Compare September 27, 2024 10:08
@antoinebhs antoinebhs requested a review from olperr1 September 27, 2024 10:10
Signed-off-by: BOUHOURS Antoine <antoine.bouhours@rte-france.com>
@antoinebhs antoinebhs force-pushed the specific-parameters-from-config branch from c20b8cc to e1e915a Compare September 27, 2024 10:21
SylvestreSakti
SylvestreSakti previously approved these changes Oct 1, 2024
@SylvestreSakti SylvestreSakti dismissed their stale review October 1, 2024 07:39

One build is failing

@antoinebhs
Copy link
Contributor Author

@SylvestreSakti, this requires the next powsybl upgrade

@SylvestreSakti
Copy link
Contributor

@SylvestreSakti, this requires the next powsybl upgrade

Yes, ok, now I understand that the checks will work with the next release of core.

Copy link
Member

@jeandemanged jeandemanged left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at powsybl/powsybl-core#3066 we should probably apply the change as well to:

  • open-security-analysis-default-parameters
  • open-sensitivityanalysis-default-parameters

right ?

@olperr1
Copy link
Member

olperr1 commented Oct 8, 2024

Looking at powsybl/powsybl-core#3066 we should probably apply the change as well to:

* `open-security-analysis-default-parameters`

* `open-sensitivityanalysis-default-parameters`

right ?

@jeandemanged: You are right... but these providers were forgotten in powsybl/powsybl-core#3133.

@olperr1
Copy link
Member

olperr1 commented Oct 8, 2024

After a double-check: SecurityAnalysisProvider and SensitivityAnalysisProvider of powsybl-core don't have a getSpecificParameter() method. This explains why getSpecificParameters(PlatformConfig platformConfigConfig) was not introduced in these interfaces and why no changes were made here for:

  • open-security-analysis-default-parameters
  • open-sensitivityanalysis-default-parameters

I will open an issue in powsybl-core on this subject, but for now these providers shouldn't be updated.

@olperr1
Copy link
Member

olperr1 commented Oct 8, 2024

I will open an issue in powsybl-core on this subject, but for now these providers shouldn't be updated.

The issue is here: powsybl/powsybl-core#3175

Copy link

@geofjamg geofjamg merged commit cc90496 into main Oct 11, 2024
7 checks passed
@geofjamg geofjamg deleted the specific-parameters-from-config branch October 11, 2024 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants