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

Add pq switch to reports [Currently on top of report test format] #1182

Open
wants to merge 10 commits into
base: report_test_format
Choose a base branch
from

Conversation

vidaldid-rte
Copy link
Collaborator

Please check if the PR fulfills these requirements

  • [ X ] The commit message follows our guidelines
  • [ X ] 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?
Report PQ/PV switchs

What is the current behavior?
Only summary of switches are reported
Details are only logged at trace level.

What is the new behavior (if this is a feature change)?
Under the summary, a list of nodes with individual changes are reported.

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

  • Yes
  • [ X ] No

Copy link
Contributor

@SylvestreSakti SylvestreSakti left a comment

Choose a reason for hiding this comment

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

Thank you. I am ok with these changes, just some details about the messages content

Base automatically changed from robustify_remote_control to main February 19, 2025 12:33
@vidaldid-rte vidaldid-rte force-pushed the add_pq_switch_top_reports branch from 1e6e550 to c1d623d Compare February 19, 2025 13:12
@vidaldid-rte vidaldid-rte changed the title [WIP] Add pq switch to reports Add pq switch to reports Feb 19, 2025
@SylvestreSakti
Copy link
Contributor

Unless Sonar that wants you to replace "busId" by the constant BUS_ID in Reports, I am ok with the PR 👍

@vidaldid-rte
Copy link
Collaborator Author

Unless Sonar that wants you to replace "busId" by the constant BUS_ID in Reports, I am ok with the PR 👍

This is not a good idea. busID is used in the string template. It is better to be able to check visually that the parameter is same as the one in the template.

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.

this PR should not have to deal with rounding.
rounding issue will be addressed by #1187.
Once we have new powsybl-core RC we will be able to finalize this PR

@vidaldid-rte
Copy link
Collaborator Author

this PR should not have to deal with rounding. rounding issue will be addressed by #1187. Once we have new powsybl-core RC we will be able to finalize this PR

Good point. I'll add the need rc-core tag and see how the reports look like with the new core behaviour.

@vidaldid-rte vidaldid-rte force-pushed the add_pq_switch_top_reports branch from 38adfa5 to f05c320 Compare March 5, 2025 11:30
@vidaldid-rte vidaldid-rte changed the base branch from main to report_test_format March 5, 2025 11:35
@vidaldid-rte vidaldid-rte changed the title Add pq switch to reports Add pq switch to reports [Currently on top of report test format] Mar 5, 2025
@vidaldid-rte
Copy link
Collaborator Author

this PR should not have to deal with rounding. rounding issue will be addressed by #1187. Once we have new powsybl-core RC we will be able to finalize this PR

@jeandemanged Done. Note that #1187 applies only to testing. The underlying core PR powsybl/powsybl-core#3317, just allows to define variable formatters when tunring reports into Strings. The formatters may take advantage of the Unit. So I added a Unit to the new messages, when it makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

3 participants