Skip to content

Observability check: Phasor voltage sensor #1024

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

Merged
merged 10 commits into from
Jun 24, 2025

Conversation

figueroa1395
Copy link
Contributor

@figueroa1395 figueroa1395 commented Jun 17, 2025

Closes #914

Checklist:

  • Implementation
  • Unit tests
  • Validation tests
  • Refactor
  • Documentation

Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
@figueroa1395 figueroa1395 self-assigned this Jun 17, 2025
@figueroa1395 figueroa1395 added the feature New feature or request label Jun 17, 2025
@figueroa1395
Copy link
Contributor Author

figueroa1395 commented Jun 17, 2025

Quality Gate Passed Quality Gate passed

Issues 1 New issue 0 Accepted issues

Measures 0 Security Hotspots 97.8% Coverage on New Code 1.0% Duplication on New Code

See analysis details on SonarQube Cloud

I purposely didn't do what sonar is suggesting as that would involve splitting the if statement in two, making it larger and less readable in my opinion. However, if there are strong opinions in favor of this, I will make the change.

Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Copy link
Contributor

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo left a comment

Choose a reason for hiding this comment

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

I am a little bit hesitant to say the current flow of handling is optimal. The reason is inhomogeneous processing of states ins the observability status. Additionally, depending on the electrical context, we might want to differentiate unobservable grids when they are not sufficiently observable and when they are not necessarily observable.

Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
@figueroa1395
Copy link
Contributor Author

figueroa1395 commented Jun 20, 2025

I am a little bit hesitant to say the current flow of handling is optimal. The reason is inhomogeneous processing of states ins the observability status. Additionally, depending on the electrical context, we might want to differentiate unobservable grids when they are not sufficiently observable and when they are not necessarily observable.

Let me know what you think after the changes. Right now we only use the observability check for two reasons:

  1. If either the necessary or sufficient conditions fail, then we throw a NotObservableError with a descriptive message.
  2. If both conditions are successful, we then use (newly named) is_observable variable to determine whether we should use perturbation or not, in conjunction with the is_possibly_ill_conditioned check.

The flow is clear in my opinion. If we were to explicitly need the sufficient and necessary conditions individually in the future, it is trivial to add them in the current state. Also, the flow is kept the same as introduced in #858.

Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Signed-off-by: Santiago Figueroa Manrique <figueroa1395@gmail.com>
Copy link

Copy link
Contributor

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo left a comment

Choose a reason for hiding this comment

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

I agree with the verdict on sonar cloud warning. Hereby approve.

@figueroa1395 figueroa1395 added this pull request to the merge queue Jun 24, 2025
Merged via the queue into main with commit a3bcd36 Jun 24, 2025
31 checks passed
@figueroa1395 figueroa1395 deleted the feature/phasor-voltage-observability branch June 24, 2025 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[FEATURE] Addition of phasor voltage sensors to the necessary and sufficient conditions for observability in radial grids
3 participants