Skip to content

Merge links into nodes: hook link algorithm - #1536

Merged
mgovers merged 33 commits into
mainfrom
pgm/feature/link-supernode-output
Aug 26, 2026
Merged

Merge links into nodes: hook link algorithm#1536
mgovers merged 33 commits into
mainfrom
pgm/feature/link-supernode-output

Conversation

@figueroa1395

@figueroa1395 figueroa1395 commented Aug 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Part of #35

Follow up of #1520

This PR activates the link solver but it doesn't hook up the output yet.

@figueroa1395
link-less implementation
d3a4dcf 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
link-less testing
5ece020 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395 figueroa1395 self-assigned this Aug 14, 2026
@figueroa1395 figueroa1395 added feature New feature or request do-not-merge This should not be merged labels Aug 14, 2026
figueroa1395 and others added 4 commits August 19, 2026 13:18
@figueroa1395
code quality
b204224 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
Merge branch 'main' into pgm/feature/link-supernode-output
8e5d0a3
@figueroa1395
activate link solver
47122b8 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
cleanup: ComplexVector
7903df9 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
mgovers and others added 7 commits August 20, 2026 09:20
@mgovers
tests for link solver output
c7159ec 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
cleanup
3d0fab7 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@figueroa1395
address comments
877fcbd 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395
futher testing of link addition
2e7989e 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@mgovers
clang-tidy + gcc
aedb8ac 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@figueroa1395
Merge branch 'main' into pgm/feature/link-supernode-output
de6b24b
@figueroa1395
format
1dc682e 
Signed-off-by: Santiago Figueroa Manrique <santiago.figueroa.manrique@alliander.com>
@figueroa1395 figueroa1395 removed the do-not-merge This should not be merged label Aug 21, 2026
@figueroa1395
figueroa1395 marked this pull request as ready for review August 21, 2026 09:32
mgovers
mgovers reviewed Aug 21, 2026
View reviewed changes
Comment thread ...grid_model_c/power_grid_model/include/power_grid_model/main_core/topological_node_output.hpp Outdated
Comment thread ...grid_model_c/power_grid_model/include/power_grid_model/main_core/topological_node_output.hpp Outdated
Comment thread ...grid_model_c/power_grid_model/include/power_grid_model/main_core/topological_node_output.hpp Outdated
Comment thread ...grid_model_c/power_grid_model/include/power_grid_model/main_core/topological_node_output.hpp Outdated
@mgovers mgovers added the do-not-merge This should not be merged label Aug 21, 2026
@mgovers

mgovers commented Aug 21, 2026

Copy link
Copy Markdown
Member

reviewed. after open comments are resolved can be merged on monday.

@figueroa1395 figueroa1395 changed the title Merge links into nodes: Create link output from link solver algorithm Merge links into nodes: hook link algorithm Aug 21, 2026
@figueroa1395 figueroa1395 mentioned this pull request Aug 21, 2026
Open
mgovers added 2 commits August 24, 2026 10:08
@mgovers
remove reference_wrapper around lambda
11e1b67 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
add link i_f and i_t to supernode output
4d72d0b 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
mgovers added 5 commits August 24, 2026 14:09
@mgovers
minor
6bb6e79 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
clang-tidy
0807584 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
remove redundant
db5adc7 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
fix doctest almost equal
ece4723 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
work around gcc issue
f2f3a38 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
Jerry-Jinfeng-Guo
Jerry-Jinfeng-Guo previously approved these changes Aug 25, 2026
View reviewed changes

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@mgovers
mgovers added this pull request to the merge queue Aug 25, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 25, 2026
@mgovers

mgovers commented Aug 25, 2026

Copy link
Copy Markdown
Member

merge conflict. i will now resolve them

@mgovers
mock component container with edge
7b3e9b4 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
mgovers enabled auto-merge August 25, 2026 13:44
@mgovers

mgovers commented Aug 25, 2026

Copy link
Copy Markdown
Member

merge conflict should be fixed. please re-review

mgovers and others added 2 commits August 26, 2026 08:26
@mgovers
clang-tidy
2a99d53 
Signed-off-by: Martijn Govers <Martijn.Govers@Alliander.com>
@mgovers
Merge branch 'main' into pgm/feature/link-supernode-output
0a16d2c 
Signed-off-by: Martijn Govers <martijn.govers@alliander.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 26, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
7 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

@marhofmann marhofmann mentioned this pull request Aug 26, 2026
Merged
@Jerry-Jinfeng-Guo

Jerry-Jinfeng-Guo commented Aug 26, 2026

Copy link
Copy Markdown
Member

What do we want to do with the first sonar cloud issue? Otherwise lgtm.

@mgovers

mgovers commented Aug 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

What do we want to do with the first sonar cloud issue? Otherwise lgtm.

it's an instance of #1401

The subtype is of type for (auto&& a : std::views::zip(x, y)) { /*...*/ }, which sonar since recently has started to understand. However, because it's now using std::ranges::for_each(std::views::zip(x, y), [](auto&& a) { /*...*/ }), which sonar doesn't understand yet.

I don't think we need to dive into this now.

Jerry-Jinfeng-Guo
Jerry-Jinfeng-Guo approved these changes Aug 26, 2026
View reviewed changes

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sonar cloud issues are either introduced somewhere else or intentional TODO's. Approve.

@mgovers
mgovers added this pull request to the merge queue Aug 26, 2026
Merged via the queue into main with commit e3a8308 Aug 26, 2026
34 checks passed
@mgovers
mgovers deleted the pgm/feature/link-supernode-output branch August 26, 2026 14:29
marhofmann added a commit to marhofmann/power-grid-model that referenced this pull request Aug 26, 2026
@marhofmann
Give per-user-node vectors inline storage
18e11da 
Each topological node holds one value per user node, and only link-merged
supernodes have more than one -- in a grid without links every topological
node has exactly one. Three of these vectors are rebuilt on every scenario
(node_injection, node_flow_from_branch and bus_injection), so each node asks
the allocator for room to store a single complex number three times per
scenario.

Give them inline room for one element, so the ordinary node does not
allocate. Link-merged nodes still work, they just allocate as before. The
indexing interface is unchanged. As a side effect the copy from
node_injection into bus_injection stops being a heap copy, because both
sides are now the same inline type.

Expose the container generically rather than at the point of use: a new
common/small_vector.hpp holds a single using declaration for SmallVector,
alongside the other type aliases in common/. Other call sites can then reach
for it without pulling in boost directly, and the underlying container can
be swapped in one place. boost::container::small_vector is header-only and
Boost::headers is already an INTERFACE dependency of the power_grid_model
target, so this adds nothing to the build. The inline capacity stays a
per-use-site decision; here it is 1 because that is exactly what an ordinary
topological node holds.

Measured on tests/benchmark_cpp (radial grid, 2605 nodes, 1000-scenario
symmetric Newton-Raphson batch), MSVC /O2, against the parent commit, 10
interleaved rounds per arm on an otherwise idle machine, medians:

  produce output    2.4487 s -> 2.2470 s   1.09x
  math calculation  2.0757 s -> 2.1361 s   control, unchanged within noise
  whole batch run   4.9217 s -> 4.7514 s   1.04x

The change won 9 of the 10 paired rounds, so the effect is real, but the
0.20 s median gain sits inside main's own 0.69 s run-to-run spread, so treat
it as modest rather than decisive.

The gain is smaller than it would have been before PowerGridModel#1536, because most of
the per-node allocation cost now sits in compute_link_solver, which this
does not touch. See the linked issue.

All ten C++ test suites pass, 176999 assertions, built with warnings as
errors, including the handling-of-links cases that exercise supernodes
holding more than one user node.

Signed-off-by: Martin Hofmann <martin.hofmann-3@ei.thm.de>
This was referenced Aug 26, 2026
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgovers mgovers mgovers left review comments
@nitbharambe nitbharambe nitbharambe left review comments
@Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo Jerry-Jinfeng-Guo approved these changes

Assignees

@figueroa1395 figueroa1395

Labels

feature New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@figueroa1395 @mgovers @Jerry-Jinfeng-Guo @nitbharambe