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

Dynamic simulation output variables supplier #914

Merged
merged 8 commits into from
Dec 17, 2024

Conversation

Lisrte
Copy link
Contributor

@Lisrte Lisrte commented Dec 12, 2024

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)
  • Docs have been added / updated (for bug fixes / features)

Does this PR already have an issue describing the problem?

No

What kind of change does this PR introduce?

Feature

What is the current behavior?

Dynamic simulation CurveMapping handle curve for dynamic models.

What is the new behavior (if this is a feature change)?
CurveMapping is replaced with OutputVariableMapping handling curves and final state values for equipment with or without dynamic models.
Final state values results are provided by SimulationResult.final_state_values.

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Replace CurveMapping::add_curve and CurveMapping::add_curve with OutputVariableMapping::add_dynamic_model_curves.

Other information:

Linked to the powsybl-dynawo PR 413
Without the PR, mapping with more than two curves will end with pandas.errors.InvalidIndexError.

@Lisrte Lisrte self-assigned this Dec 12, 2024
Add fsv input

Signed-off-by: lisrte <laurent.issertial@rte-france.com>
Signed-off-by: lisrte <laurent.issertial@rte-france.com>
Signed-off-by: lisrte <laurent.issertial@rte-france.com>
@Lisrte Lisrte force-pushed the dynamic_output_variables branch from 482385b to 468d08c Compare December 12, 2024 11:45
Signed-off-by: lisrte <laurent.issertial@rte-france.com>
Signed-off-by: lisrte <laurent.issertial@rte-france.com>
Signed-off-by: lisrte <laurent.issertial@rte-france.com>
@Lisrte Lisrte marked this pull request as ready for review December 12, 2024 15:10
@Lisrte Lisrte requested a review from HugoKulesza December 12, 2024 15:10
@@ -37,4 +38,8 @@ def _get_all_curves(self) -> pd.DataFrame:
curve_name_lst = _pp.get_all_dynamic_curves_ids(self._handle)
df_curves = [self._get_curve(curve_name)
for curve_name in curve_name_lst]
return pd.concat(df_curves, axis=1) if df_curves else pd.DataFrame()
return pd.concat(df_curves, axis=1).ffill() if df_curves else pd.DataFrame()
Copy link
Collaborator

Choose a reason for hiding this comment

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

By curiosity, why is .ffill needed here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Curves are represented as a map of DoubleTimeSeries in Powsybl. When creating a DoubleTimeSeries for an individual curve, entries with the same value as the previous one are skipped. Thus when we concat curves series we have to fill forward in order to get these values back.

pypowsybl/dynamic/impl/output_variable_mapping.py Outdated Show resolved Hide resolved
integration_tests/test_dynawo.py Outdated Show resolved Hide resolved
Lisrte and others added 2 commits December 17, 2024 13:55
Signed-off-by: lisrte <laurent.issertial@rte-france.com>
Copy link

Copy link

@HugoKulesza HugoKulesza merged commit 2a2a49d into main Dec 17, 2024
9 checks passed
@HugoKulesza HugoKulesza deleted the dynamic_output_variables branch December 17, 2024 13:46
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.

2 participants