feat: support merge of two grids #136

wwadman · 2026-01-13T14:37:56Z

Fixes issue: #118

Changes proposed in this PR include:

Adding function Grid.merge(self, other_grid) that merges another grid into this grid.

I went for the hardcoded approach as explained in [FEATURE] Merge grids #118 (comment)
Ideally we should make this more general, for example by adding an indicator ID_COLUMN whether a column contains ids or not.

Could you please pay extra attention to the points below when reviewing the PR:

~~Search for Question for reviewer in the diff~~

Checks

I added some tests that in my view cover most of the workings.

If changes are related to CI/CD, are they verified via a manual run on this branch?

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

I, wander.wadman <wander.wadman@alliander.com>, hereby add my Signed-off-by to this commit: e0fe4f2 Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

src/power_grid_model_ds/_core/model/grids/_helpers.py

tests/unit/model/grids/test_modify.py

src/power_grid_model_ds/_core/model/grids/_helpers.py

Signed-off-by: wander.wadman <wander.wadman@alliander.com> # Conflicts: # src/power_grid_model_ds/_core/model/grids/_helpers.py # src/power_grid_model_ds/_core/model/grids/base.py # tests/unit/model/grids/test_modify.py

Signed-off-by: Thijs Baaijen <13253091+Thijss@users.noreply.github.com>

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Signed-off-by: jaapschoutenalliander <jaap.schouten@alliander.com>

src/power_grid_model_ds/_core/model/grids/_helpers.py

jaapschoutenalliander

Thanks for the good work

Signed-off-by: jaapschoutenalliander <jaap.schouten@alliander.com>

src/power_grid_model_ds/_core/model/grids/_helpers.py

vincentkoppen

Some small remarks, but looks good overall!
Thanks for the PR

src/power_grid_model_ds/_core/model/grids/base.py

vincentkoppen · 2026-01-22T09:05:00Z

src/power_grid_model_ds/_core/model/grids/base.py

+        """Merge another grid into this grid. When ids overlap, ids of other_grid are offset to avoid conflicts.
+
+        Args:
+            other_grid (G): The grid to merge into this grid.
+            mode (str): The merge mode:
+                - "recalculate_ids": Recalculate ids of other_grid to avoid conflicts.
+                - "keep_ids": Keep ids of other_grid. Raises an error if grids contain overlapping indices.
+        """


Update docstring to explicitely mention the current hardcoded support.

Suggested change

"""Merge another grid into this grid. When ids overlap, ids of other_grid are offset to avoid conflicts.

Args:

other_grid (G): The grid to merge into this grid.

mode (str): The merge mode:

- "recalculate_ids": Recalculate ids of other_grid to avoid conflicts.

- "keep_ids": Keep ids of other_grid. Raises an error if grids contain overlapping indices.

"""

"""Merge another grid into this grid.

Args:

other_grid (G): The grid to merge into this grid.

mode (str): The merge mode:

- "recalculate_ids": ids in the arays of other_grid are offset to avoid conflicts.

IMPORTANT: we currently only update any `id` column and all id references in the default PGM-DS grid.

- "keep_ids": Keep ids of other_grid. Raises an error if grids contain overlapping indices.

"""

vincentkoppen · 2026-01-22T09:06:26Z

src/power_grid_model_ds/_core/model/grids/base.py

        )
        return save_grid_to_pickle(self, cache_dir=cache_dir, cache_name=cache_name, compress=compress)

+    def merge(self, other_grid: G, mode: str) -> None:


Type hinting wise, we now allow any Grid. However I think that here we want it to be the same as the current grid.
So I think this should be Self?

vincentkoppen · 2026-01-22T09:13:34Z

src/power_grid_model_ds/_core/model/grids/_helpers.py

    return grid_class(**empty_fields)
+
+
+def merge_grids(grid: G, other_grid: G, mode: str) -> None:


Also use the literal here.

vincentkoppen · 2026-01-22T09:16:41Z

src/power_grid_model_ds/_core/model/grids/_helpers.py

+            case SymGenArray() | SymLoadArray() | SourceArray():
+                columns = ["node"]
+            case _:
+                raise NotImplementedError(f"The array of type {type(array)} is not implemented for appending")


We could maybe add an call for action?
To signal that we are willing to think about a more general solution if this is needed by anyone?

Something along the line of:

Suggested change

raise NotImplementedError(f"The array of type {type(array)} is not implemented for appending")

raise NotImplementedError(f"The array of type {type(array)} is not implemented for appending. Let us know if more general support is needed.")

vincentkoppen · 2026-01-22T09:23:24Z

tests/unit/model/grids/test_helpers.py

+    def test_merge_two_grids(self):
+        grid1 = Grid.from_txt("S1 2", "S1 3 link", "3 4 transformer")
+        grid2 = Grid.from_txt("S11 12", "S11 13 link", "13 14 transformer")
+        grid1_size = grid1.node.size
+        grid2_size = grid2.node.size
+
+        grid1.merge(grid2, mode="keep_ids")
+        merged_grid_size = grid1.node.size
+
+        assert merged_grid_size == grid1_size + grid2_size, "Merged grid size should be the sum of both grids' sizes"


Also test that we didn't alter the ids. I'm also in facor of more direct values if the example is small enough, so we could do something like below.

So something like below?

Suggested change

def test_merge_two_grids(self):

grid1 = Grid.from_txt("S1 2", "S1 3 link", "3 4 transformer")

grid2 = Grid.from_txt("S11 12", "S11 13 link", "13 14 transformer")

grid1_size = grid1.node.size

grid2_size = grid2.node.size

grid1.merge(grid2, mode="keep_ids")

merged_grid_size = grid1.node.size

assert merged_grid_size == grid1_size + grid2_size, "Merged grid size should be the sum of both grids' sizes"

def test_merge_two_grids(self):

grid1 = Grid.from_txt("S1 2", "S1 3 link", "3 4 transformer")

grid2 = Grid.from_txt("S11 12", "S11 13 link", "13 14 transformer")

grid1.merge(grid2, mode="keep_ids")

assert grid1.node.id.tolist() == [1,2,3,4,11,12,13], "Merged node ids should be equal to those of grid1 and grid2 combined"

vincentkoppen · 2026-01-22T09:30:35Z

tests/unit/model/grids/test_helpers.py

+        )
+
+        # assert node in grid.source is updated by checking if the node column contains values that are all node ids:
+        assert set(grid1.source.node).issubset(grid1.node.id), "All source nodes should be valid node ids!"


nit:
Same here, I would prefer hardcoded values since the example is small enough. This removes the complexity from the test.

We now the grid_counter should be 501 (or maybe 502?)
So than it becomes like (have not updated all checks)

grid1 = Grid.from_txt("S1 2", "S1 3 link", "3 4 transformer") grid2 = Grid.from_txt("S1 2", "S1 13 link", "13 14 transformer") source = SourceArray(id=[501], node=[1], status=[1], u_ref=[0.0]) grid1.append(source) grid2.append(source) grid1.merge(grid2, mode="recalculate_ids") assert grid1.check_ids() is None, "Asset ids are not unique after merging!" expected_offset = 501 assert grid1.node.id = [1, 2, 3, 4, expected_offset +1, expected_offset + 2, expected_offset + 13,expected_offset + 14] assert grid1.link.from_node = [.....]

vincentkoppen · 2026-01-22T09:31:26Z

tests/unit/model/grids/test_helpers.py

+    def test_merge_two_grids_with_overlapping_line(self):
+        # Now both grids have 14 as highest node id, so both will have branch ids 15, 16 and 17:
+        grid1 = Grid.from_txt("S1 2", "S1 3 link", "3 14 transformer")
+        grid2 = Grid.from_txt("S1 2", "S1 13 link", "13 14 transformer")
+
+        grid1.merge(grid2, mode="recalculate_ids")
+        assert grid1.check_ids() is None, "Asset ids are not unique after merging!"


Don't see what this tests?
Missing asserts on the ids?

vincentkoppen · 2026-01-22T09:32:17Z

tests/unit/model/grids/test_helpers.py

+from power_grid_model_ds._core.model.arrays import SourceArray
+
+
+class TestMergeGrids:


Add a test that we give a runtime error if we have an ExtendedGrid with a new array type and use recalculate_ids.

vincentkoppen · 2026-01-22T09:34:57Z

tests/unit/model/grids/test_helpers.py

+from power_grid_model_ds._core.model.arrays import SourceArray
+
+
+class TestMergeGrids:


Test not implemented error for invalid call to the merge function. (so mode="invalid" in the call)

…grid Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

wwadman added 11 commits January 8, 2026 14:37

test commit

e0fe4f2

fixing too long line

8e121f9

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

after visionary talk with Thijs

ae4d72b

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

iets doet t :)

8b5e693

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

grids seem ok when eyeballing in Graph Editor

402f835

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Second test grid ready to be tested

7e79786

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Another test

78c4788

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Testing removal of duplicate line ids

4b2cf47

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

SOme formatting

2ddf464

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

poe format

60068b4

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Cleaner code

e7fbf65

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

wwadman marked this pull request as ready for review January 13, 2026 14:51

wwadman marked this pull request as draft January 13, 2026 14:51

wwadman added 2 commits January 13, 2026 16:28

Poe all and questions removed

7c5a798

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

DCO Remediation Commit for wander.wadman <wander.wadman@alliander.com>

7df3173

I, wander.wadman <wander.wadman@alliander.com>, hereby add my Signed-off-by to this commit: e0fe4f2 Signed-off-by: wander.wadman <wander.wadman@alliander.com>

vincentkoppen changed the title ~~Feat/merge grids~~ feat: support merge of two grids Jan 14, 2026

vincentkoppen mentioned this pull request Jan 14, 2026

draft: metadata support for columns #138

Draft

wwadman added 2 commits January 14, 2026 16:51

add mode argument

db61e42

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Counter empty columns

16e837a

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

wwadman commented Jan 15, 2026

View reviewed changes

Merge remote-tracking branch 'origin/main' into feat/merge-grids

e9d7c36

Signed-off-by: wander.wadman <wander.wadman@alliander.com> # Conflicts: # src/power_grid_model_ds/_core/model/grids/_helpers.py # src/power_grid_model_ds/_core/model/grids/base.py # tests/unit/model/grids/test_modify.py

wwadman marked this pull request as ready for review January 15, 2026 13:43

Thijss and others added 2 commits January 21, 2026 14:13

Merge branch 'main' into feat/merge-grids

de0241d

Signed-off-by: Thijs Baaijen <13253091+Thijss@users.noreply.github.com>

Resolving test failing

ffb59ef

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

wwadman marked this pull request as draft January 21, 2026 14:09

wwadman added 2 commits January 21, 2026 15:17

Resolving mypy src tests

d9f5c75

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Resolving last mypy src test

2e03fe5

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Thijss marked this pull request as ready for review January 21, 2026 14:47

wwadman added 2 commits January 21, 2026 15:50

Removing comment

88d86eb

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Moved tests to test_helpers

1add36c

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

wwadman and others added 3 commits January 21, 2026 15:55

poe all formatting

b7cfa78

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

License added

ee598d6

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

feat: add check ids

5af0b51

Signed-off-by: jaapschoutenalliander <jaap.schouten@alliander.com>

jaapschoutenalliander reviewed Jan 21, 2026

View reviewed changes

src/power_grid_model_ds/_core/model/grids/_helpers.py Show resolved Hide resolved

jaapschoutenalliander approved these changes Jan 21, 2026

View reviewed changes

chore: format

cab74ed

Signed-off-by: jaapschoutenalliander <jaap.schouten@alliander.com>

jaapschoutenalliander reviewed Jan 22, 2026

View reviewed changes

src/power_grid_model_ds/_core/model/grids/_helpers.py Outdated Show resolved Hide resolved

vincentkoppen requested changes Jan 22, 2026

View reviewed changes

wwadman added 2 commits January 22, 2026 15:59

Only deepcopy arrays of the second grid instead of the entire second …

c86efff

…grid Signed-off-by: wander.wadman <wander.wadman@alliander.com>

Make mode a Literal

f2babcb

Signed-off-by: wander.wadman <wander.wadman@alliander.com>

		return grid_class(**empty_fields)


		def merge_grids(grid: G, other_grid: G, mode: str) -> None:

	raise NotImplementedError(f"The array of type {type(array)} is not implemented for appending")
	raise NotImplementedError(f"The array of type {type(array)} is not implemented for appending. Let us know if more general support is needed.")

		from power_grid_model_ds._core.model.arrays import SourceArray


		class TestMergeGrids:

feat: support merge of two grids #136

Are you sure you want to change the base?

feat: support merge of two grids #136

Conversation

wwadman commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes proposed in this PR include:

Could you please pay extra attention to the points below when reviewing the PR:

Checks

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jaapschoutenalliander left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vincentkoppen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vincentkoppen Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

vincentkoppen Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wwadman commented Jan 13, 2026 •

edited

Loading

vincentkoppen left a comment •

edited

Loading

vincentkoppen Jan 22, 2026 •

edited

Loading

vincentkoppen Jan 22, 2026 •

edited

Loading

vincentkoppen Jan 22, 2026 •

edited

Loading

vincentkoppen Jan 22, 2026 •

edited

Loading