Skip to content

Clean-up main model: move topology related construction #1092

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
Merged
merged 17 commits into from
Aug 26, 2025
Merged

Clean-up main model: move topology related construction #1092

merged 17 commits into from
Aug 26, 2025

Conversation

nitbharambe
Copy link
Member

@nitbharambe nitbharambe commented Aug 19, 2025 •
edited
Loading

In the large effort of refactoring main model, one part is to move the topology and y bus related construction to a different place.

  • Moved topology logic to main_core/topology.hpp
  • Changed topology's dependency from state to component container.
  • Refactored ComponentConnections construction in similar way as ComponentTopology in 1ab7ee8
  • Moved all small individual topology functions in main_core/topology.hpp to detail namespace.
  • Edited component_container_c to include more container functionality and applicable for 1+ component type. Moved it to power_grid_model::common namespace

@nitbharambe
move construct topology
e6728db 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe nitbharambe changed the title move construct topology Refactor topology/ybus: move topology related construction Aug 19, 2025
@nitbharambe nitbharambe self-assigned this Aug 19, 2025
@nitbharambe nitbharambe added the improvement Improvement on internal implementation label Aug 19, 2025
@nitbharambe nitbharambe changed the title Refactor topology/ybus: move topology related construction Refactor topology / ybus part of main model: move topology related construction Aug 19, 2025
@nitbharambe
address sonar cloud
0c065ea 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
decouple topology related state queries
df0dcbf 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
shift to component container
3d29f46 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo changed the title Refactor topology / ybus part of main model: move topology related construction Clean-up main model: move topology / ybus related construction Aug 20, 2025
@nitbharambe
move concepts to container_fwd
68ebe00 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
move out container queries
dd65f3b 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
change size retrieval to component query
8597a46 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
decouple topology from state
fbbaa5e 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
homogenize registration
1ab7ee8 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
Copy link
Member Author

Need thoughts on 1ab7ee8. Should we keep or revert the commit?

We had earlier decided against it when it was dependent on state and lived in main model. Reason was that this function is small unlike construct_topology. In the context of current PR, we would be homogenizing both these functions.

@mgovers
Copy link
Member

mgovers commented Aug 20, 2025

Need thoughts on 1ab7ee8. Should we keep or revert the commit?

We had earlier decided against it when it was dependent on state and lived in main model. Reason was that this function is small unlike construct_topology. In the context of current PR, we would be homogenizing both these functions.

i kind of like the commit, so IMO you can keep it

@nitbharambe
rename common function to generic name
317c8ed 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
resolve todos
423dcd2 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
rename to retrievable type
5937456 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe nitbharambe changed the title Clean-up main model: move topology / ybus related construction Clean-up main model: move topology related construction Aug 20, 2025
@nitbharambe
Copy link
Member Author

Limiting this PR's scope. Continuing in new one.

@nitbharambe nitbharambe marked this pull request as ready for review August 20, 2025 13:52
mgovers
mgovers reviewed Aug 21, 2025
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.

awesome cleanup. A couple small nitpicks but nothing major

@nitbharambe
address comments 1
ca5ae3f 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
move to detail namespace
f339e21 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
add to common namespace
0711e10 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
merge both concepts to one
077f892 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@nitbharambe
workaround compiler bug and address end()
79c1da4 
Signed-off-by: Nitish Bharambe <nitish.bharambe@alliander.com>
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
3 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

mgovers
mgovers approved these changes Aug 26, 2025
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.

no more remarks from my end. feel free to merge whenever

@nitbharambe nitbharambe added this pull request to the merge queue Aug 26, 2025
Merged via the queue into main with commit 776d3c2 Aug 26, 2025
31 checks passed
@nitbharambe nitbharambe deleted the feture/refactor-topology-main-model branch August 26, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgovers mgovers mgovers approved these changes

Assignees

@nitbharambe nitbharambe

Labels
improvement Improvement on internal implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@nitbharambe @mgovers