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.

Sign up for GitHub

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

Jump to bottom

Feature / Binary search in Automatic Tap Changer #668

Merged
merged 59 commits into from
Aug 6, 2024
Merged

Feature / Binary search in Automatic Tap Changer #668

merged 59 commits into from
Aug 6, 2024

Conversation

Jerry-Jinfeng-Guo
Copy link
Contributor

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo commented Jul 9, 2024 •
edited
Loading

This PR adds binary search to the tap changer optimization process. The follow tasks are carried out:

  • Add fast_any tap changing strategy
  • Unit test benchmark the number of runs
  • Implement binary search in the fast_any tap changing strategy
  • Implement binary search in the global_max and global_min tap changing strategy
  • Existing unit tests
  • Existing validation tests

@Jerry-Jinfeng-Guo
branch init
699f8a9 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added the feature New feature or request label Jul 9, 2024
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo self-assigned this Jul 9, 2024
Jerry-Jinfeng-Guo and others added 20 commits July 9, 2024 13:26
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
29ee28d
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
d6e362f
@Jerry-Jinfeng-Guo
added fast_any
1e81cd4 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
[skip ci] intermediate commit
2608b23 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
4d0b142
@Jerry-Jinfeng-Guo
[skip ci] added binary search members
8b942fb 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
minor clean
623f6a4 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
69dc30f
@Jerry-Jinfeng-Guo
basic logic in place
51392aa 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
e8a5315
@Jerry-Jinfeng-Guo
clean up
096ab0f 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
compiler complains
cd59565 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
90e6b1d
@Jerry-Jinfeng-Guo
[skip ci] binary search logic moved away from wrapper; debugging
bed2dff 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
5353991
@Jerry-Jinfeng-Guo
1 werror fix
6472cd1 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
3 more werror
56a8c0e 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
[skip ci] wip
a0319be 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
single transformer finished; TODO pack the indices for the binary_sea…
dabd3e1 
…rch_

Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
binary search implementation + tests done; next to add a benchmark un…
663de80 
…it test

Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
cleaned up a bit
b5ce206 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
sonar cloud
8c3d25d 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo marked this pull request as ready for review July 25, 2024 15:14
@Jerry-Jinfeng-Guo
removed SearchMethod from python enum; optimizer now assigns search m…
ba8da6e 
…ethod given strategy if not provided, which is the default case through the api; docs updated

Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
Copy link
Contributor Author

Commenting here, so the PR won't accidentally be merged. There are still some discussions going on about the Python API. Those need to be resolved before this PR is ready

Done. Actually commenting here won't prevent accidental merging ;)

@Jerry-Jinfeng-Guo
revert changes written to the notebook
270b5d5 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
clang-tidy
664796a 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
resolved one code smell
caeee05 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
docs.
612dd8c 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
mgovers
mgovers reviewed Aug 5, 2024
View reviewed changes
docs/user_manual/calculations.md Outdated Show resolved Hide resolved
docs/user_manual/calculations.md Outdated Show resolved Hide resolved
docs/user_manual/calculations.md Outdated Show resolved Hide resolved
power_grid_model_c/power_grid_model/include/power_grid_model/common/common.hpp Outdated Show resolved Hide resolved
..._grid_model_c/power_grid_model/include/power_grid_model/optimizer/tap_position_optimizer.hpp Outdated Show resolved Hide resolved
src/power_grid_model/core/error_handling.py Outdated Show resolved Hide resolved
tests/cpp_unit_tests/test_optimizer.hpp Outdated Show resolved Hide resolved
tests/data/power_flow/automatic-tap-regulator/pgm-automatic-tap-line-drop-min/params.json Outdated Show resolved Hide resolved
tests/data/power_flow/automatic-tap-regulator/pgm-automatic-tap-min/params.json Outdated Show resolved Hide resolved
docs/user_manual/calculations.md Show resolved Hide resolved
Jerry-Jinfeng-Guo and others added 6 commits August 5, 2024 15:32
@Jerry-Jinfeng-Guo
addressing most of the comments from 05-aug
4781c39 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
[skip ci] included the other proposed comment
a930bc0 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
addressing final comments from 05-aug
7aa5bc4 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
Merge branch 'main' into feature/binary-search-tap-changer
14c4606 
Signed-off-by: Jerry Guo <6221579+Jerry-Jinfeng-Guo@users.noreply.github.com>
@Jerry-Jinfeng-Guo
fotmat
3df1cf3 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@Jerry-Jinfeng-Guo
unused import
3ed8aed 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
@mgovers
Copy link
Member

mgovers commented Aug 6, 2024

remaining sonar cloud issues can be ignored

mgovers
mgovers reviewed Aug 6, 2024
View reviewed changes
Copy link
Member

@mgovers mgovers left a comment

Choose a reason for hiding this comment

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

only the comments on the example notebook needs resolution, for the rest, i'm OK with closing with a comment

docs/examples/Transformer Examples.ipynb Show resolved Hide resolved
docs/examples/Transformer Examples.ipynb Outdated Show resolved Hide resolved
docs/examples/Transformer Examples.ipynb Outdated Show resolved Hide resolved
docs/examples/Transformer Examples.ipynb Outdated Show resolved Hide resolved
docs/examples/Transformer Examples.ipynb Show resolved Hide resolved
..._grid_model_c/power_grid_model/include/power_grid_model/optimizer/tap_position_optimizer.hpp Outdated Show resolved Hide resolved
..._grid_model_c/power_grid_model/include/power_grid_model/optimizer/tap_position_optimizer.hpp Show resolved Hide resolved
@Jerry-Jinfeng-Guo
updated the notebook and part of the code.
4674159 
Signed-off-by: Jerry Guo <Jerry.Jinfeng.Guo@alliander.com>
mgovers
mgovers approved these changes Aug 6, 2024
View reviewed changes
Copy link
Member

@mgovers mgovers left a comment

Choose a reason for hiding this comment

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

1 small nitpick. feel free to ignore it. I hereby approve

docs/examples/Transformer Examples.ipynb Show resolved Hide resolved
docs/examples/Transformer Examples.ipynb Show resolved Hide resolved
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Aug 6, 2024

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
93.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo added this pull request to the merge queue Aug 6, 2024
Merged via the queue into main with commit 75c024f Aug 6, 2024
26 checks passed
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo deleted the feature/binary-search-tap-changer branch August 6, 2024 09:28
mgovers added a commit that referenced this pull request Aug 6, 2024
@mgovers
remove erroneously added test logic after #668
9f11250 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers mgovers mentioned this pull request Aug 6, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Aug 6, 2024
@mgovers
Merge pull request #684 from PowerGridModel/feature/partial-revert-bi…
82168c6 
…nary-search

remove erroneously added unused and test logic after #668
@mgovers mgovers mentioned this pull request Nov 5, 2024
Closed
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@petersalemink95 petersalemink95 petersalemink95 left review comments

@figueroa1395 figueroa1395 figueroa1395 left review comments

@mgovers mgovers mgovers approved these changes

@TonyXiang8787 TonyXiang8787 Awaiting requested review from TonyXiang8787

@nitbharambe nitbharambe Awaiting requested review from nitbharambe

Assignees

@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo

Labels
feature New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Jerry-Jinfeng-Guo @mgovers @petersalemink95 @figueroa1395 @nitbharambe