Skip to content

Basic PGM support for creating current sensors (experimental in state estimation) #936

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
Apr 3, 2025

Conversation

mgovers
Copy link
Member

@mgovers mgovers commented Mar 26, 2025

Make sure that the C++ core and Python wrapper can handle current sensors.

Relates to #547 .

This feature is experimental. The behavior is as follows:

  • If experimental features are disabled and there are current sensors in the grid and state estimation is run
    • Raises InvalidArguments with the message State estimation is not implemented for current sensors!
  • Otherwise
    • Runs the state estimation as usual while ignoring any current sensors
    • Note that this may result in a NotObservableError if there are not enough voltage and power sensors, even if the system is observable if current sensors are taken into account. This will be resolved in follow-up PRs

mgovers added 2 commits March 26, 2025 08:16
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers added the feature New feature or request label Mar 26, 2025
@mgovers mgovers self-assigned this Mar 26, 2025
@mgovers mgovers changed the title Feature/current sensor main model Basic PGM support for creating current sensors (experimental in state estimation) Mar 26, 2025
mgovers and others added 4 commits March 26, 2025 09:40
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
…m/PowerGridModel/power-grid-model into feature/current-sensor-main-model

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers marked this pull request as draft March 26, 2025 09:50
@mgovers
Copy link
Member Author

mgovers commented Mar 26, 2025

found a segfault that requires attention

EDIT: fixed in 24241ed

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers marked this pull request as ready for review March 26, 2025 10:40
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers enabled auto-merge March 26, 2025 12:08
@mgovers mgovers added the do-not-merge This should not be merged label Mar 26, 2025
@mgovers
Copy link
Member Author

mgovers commented Mar 26, 2025

Do not merge label temporarily added so that we can merge #908

EDIT: Removed after successful merge of #908 and #937

@mgovers mgovers removed the do-not-merge This should not be merged label Mar 26, 2025
@TonyXiang8787 TonyXiang8787 requested a review from Copilot March 27, 2025 12:49
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces experimental support for current sensors in both the C++ core and Python wrapper for state estimation. It adds enum entries for symmetric and asymmetric current sensors.

  • Added enum entries for symmetric and asymmetric current sensors to dataset_definitions.py
  • Supports experimental behavior for handling current sensors in state estimation
Files not reviewed (19)
  • code_generation/data/dataset_class_maps/dataset_definitions.json: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/all_components.hpp: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/calculation_parameters.hpp: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/main_core/input.hpp: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/main_core/output.hpp: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/main_core/topology.hpp: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/main_model.hpp: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/main_model_impl.hpp: Language not supported
  • power_grid_model_c/power_grid_model/include/power_grid_model/topology.hpp: Language not supported
  • power_grid_model_c/power_grid_model_c/include/power_grid_model_c/dataset_definitions.h: Language not supported
  • power_grid_model_c/power_grid_model_c/src/dataset_definitions.cpp: Language not supported
  • power_grid_model_c/power_grid_model_c/src/model.cpp: Language not supported
  • tests/data/state_estimation/basic-current-sensor/input.json: Language not supported
  • tests/data/state_estimation/basic-current-sensor/input.json.license: Language not supported
  • tests/data/state_estimation/basic-current-sensor/params.json: Language not supported
  • tests/data/state_estimation/basic-current-sensor/params.json.license: Language not supported
  • tests/data/state_estimation/basic-current-sensor/sym_output.json: Language not supported
  • tests/data/state_estimation/basic-current-sensor/sym_output.json.license: Language not supported
  • tests/native_api_tests/test_api_model.cpp: Language not supported
Comments suppressed due to low confidence (2)

src/power_grid_model/_core/dataset_definitions.py:74

  • [nitpick] Consider reordering the enum entries to follow a consistent order (e.g., alphabetical) to enhance code readability.
sym_current_sensor = "sym_current_sensor"

src/power_grid_model/_core/dataset_definitions.py:75

  • [nitpick] Consider reordering the enum entries to follow a consistent order (e.g., alphabetical) to enhance code readability.
asym_current_sensor = "asym_current_sensor"

@PowerGridModel PowerGridModel deleted a comment from mgovers Mar 28, 2025
@mgovers
Copy link
Member Author

mgovers commented Apr 1, 2025

@TonyXiang8787 you requested copilot but it's not properly set-up, cfr:

  • power_grid_model_c/power_grid_model/include/power_grid_model/all_components.hpp: Language not supported

  • power_grid_model_c/power_grid_model/include/power_grid_model/calculation_parameters.hpp: Language not supported

  • power_grid_model_c/power_grid_model/include/power_grid_model/main_core/input.hpp: Language not supported

  • power_grid_model_c/power_grid_model/include/power_grid_model/main_core/output.hpp: Language not supported

  • power_grid_model_c/power_grid_model/include/power_grid_model/main_core/topology.hpp: Language not supported

  • power_grid_model_c/power_grid_model/include/power_grid_model/main_model.hpp: Language not supported

  • power_grid_model_c/power_grid_model/include/power_grid_model/main_model_impl.hpp: Language not supported

  • power_grid_model_c/power_grid_model/include/power_grid_model/topology.hpp: Language not supported

  • power_grid_model_c/power_grid_model_c/include/power_grid_model_c/dataset_definitions.h: Language not supported

  • power_grid_model_c/power_grid_model_c/src/dataset_definitions.cpp: Language not supported

  • power_grid_model_c/power_grid_model_c/src/model.cpp: Language not supported

Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers requested a review from figueroa1395 April 3, 2025 06:01
@mgovers mgovers added this pull request to the merge queue Apr 3, 2025
Merged via the queue into main with commit 40f2849 Apr 3, 2025
30 checks passed
@mgovers mgovers deleted the feature/current-sensor-main-model branch April 3, 2025 12:53
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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants