Skip to content

refactor: Graph & IterableGraph TOML roundtrip - #2559

Merged
RobBuchananCompPhys merged 11 commits into
dissolve2/node-test-fixturesfrom
dissolve2/iterable-graph-round-trip
Jul 27, 2026
Merged

refactor: Graph & IterableGraph TOML roundtrip#2559
RobBuchananCompPhys merged 11 commits into
dissolve2/node-test-fixturesfrom
dissolve2/iterable-graph-round-trip

Conversation

@RobBuchananCompPhys

@RobBuchananCompPhys RobBuchananCompPhys commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

There's a couple of things going on in this PR, all with the purpose of fixing deserialisation of IterableGraph from TOML:

  • The problem was in the first instance that deserialisation builds out the graph in the 'opposite' direction to how we typically construct them manually*, such that when it comes to parsing a 'sub'-graph (such as any graph containing an instance of IterableGraph), the edges that depend on external nodes connected to the sub-InputsNode basically fail to materialise, since they have no awareness of any such data source. Upon closer analysis it was found that this was a problem occuring for deserialisation of any Graph node, iterable or otherwise. This issue has been fixed in this PR, so a TOML roundtrip is possible for the SubGraph test. This exact problem would have been the root cause of the IterableGraph loop edges not being parsed, however...
  • ...it turns out that no specialised de/serialisation code had ever been written for IterableGraph and LoopEdge classes, so the loopEdges_ were never parsed anyway. This has also been fixed, and there are new tests for this.

*it should be noted that after implementing these changes, it should in theory be possible for a user to manually construct a nested graph from the inside-out without having yet emplaced the relevant external nodes! It may however require some further GUI work where, for instance, hovering an edge over a graph's InputsNode triggers the edge creation, whether or not any (proxy) inputs exist or not.

@RobBuchananCompPhys
RobBuchananCompPhys marked this pull request as ready for review July 23, 2026 13:12

@rprospero rprospero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is going to conflict with my #2560, but I can do a rebase.

@trisyoungs trisyoungs 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.

One heavily suggested change, otherwise looks OK.

Comment thread src/nodes/edge.cpp
@RobBuchananCompPhys
RobBuchananCompPhys merged commit 87ad8f6 into dissolve2/node-test-fixtures Jul 27, 2026
@RobBuchananCompPhys
RobBuchananCompPhys deleted the dissolve2/iterable-graph-round-trip branch July 27, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants