Problem with FindVertexGroups() and registered vertices

[jgoeders]

During construction of a DelayGraphSolution(), FixSolutionForSiblingVertices() is called, which ensures that for all registered vertices, their siblings are also registered. This process relies upon FindVertexGroups() to determine groupings of sibling vertices.

I am running into inconsistencies between two scenarios in which I am creating a DelayGraphSolution():

1. Using the NI-created RegisterPlacement code. This code executes a register placement algorithm to obtain a set of registeredTerminals. It then creates a DelayGraphSolution, passing in the set of registeredTerminals. The FindVertexGroups() code correctly finds all sibling groups, and siblings of vertices in the registeredTerminals set are correctly marked as registered.

2. My own flow. This code executes a register placement algorithm to obtain a set of registeredTerminals. In the graph these are marked as isRegistered, and the graph is written back out to a graphMl file. This file is then imported back into the DelayGraph structures, and a DelayGraphSolution is constructed. This differs from the first flow because now the registered vertices are already marked as properties in the graph, rather than in the separate registeredTerminals set. Under this flow, FindVertexGroups() will behave differently, and will fail to return the siblings of the nodes that have the property isRegistered.

FindVertexGroups() contains the following code comment:

// group vertices which are inputs and outputs of common node ids together
// there are 3 types of vertices: IsRegistered, (!IsRegistered && IsInputTerminal), (!IsRegisted && !IsInputTerminal)
// ignore IsRegistered vertices, and group the other types by common node id and types

So it appears that the function is intentionally ignoring the vertices that are marked as isRegistered. Is this a bug? If this is done intentionally, how can I go about my flow, where a graph is exported, then imported, and obtain the same results as the NI provided code?

[Tai78746]

A better name for this property is probably, "IsInternalRegister".
The original intent for this was to model internal registers, which cannot be changed to non-registers.
So that a 3 cycle multiple would have, for example, x and y input terminal vertices (marked as !IsRegistered && IsInputTerminal), 3 internal register vertices (marked as "IsRegistered"), and one output terminal marked as (!IsRegistered && !IsInputTerminal).

When you read back in your solution, you should not change this IsRegistered property (which is much clearer if we renamed it IsInternalRegister property).

[jgoeders]

Thanks! So is there a different property I should be using? Or do I need to create my own to use? If I create my own I will probably rename isRegistered to isInternalRegister as you suggest, and then use isRegistered as a new property.

[adamarnesen]

I think you should make that change to the name. The tricky thing is that you'll have to mutate all of the existing GraphML to make the properties de-serialize properly.

[adamarnesen]

The other way you could do this is to actually just have another file along side of the GraphML that is just going to have a list of nodeID that you want to add registers to as part of the solution. That will avoid the issue of having to update all the existing data sets.