Implement unique id for vertices in a transpose flow graph #229
Conversation
Entenwilli
added
enhancement
Entenwilli
requested review from
sebinside,
Nicolas-Boltz,
01Parzival10,
BenjaminArp and
uuqjz
| uuidString.append(previousElement.getUniqueIdentifier()); | ||
| } | ||
| return UUID.nameUUIDFromBytes(uuidString.toString().getBytes(StandardCharsets.UTF_8)); | ||
| } |
There was a problem hiding this comment.
This is not enough to uniquely identify the vertex.
Assume a DFD like this:
B
Z - A < > D
C
This would mean 2 partial TFG's ZABD and ZACD (assuming both flows land in the same pin of D).
It is impossible to differentiate between the Z/A vertices in the different partial flow graphs with the information contained in the Class. This is always the case if they share the same sequence of previous vertices.
It's fine if there is a different sequence of previous vertices (use of different pin, loop, etc.).
However if they share the same sequence they are equal in everything (previous vertices sequence/ data/node characteristics), so maybe it's fine if they share an ID?
Otherwise, the only difference is in which Partial TFG they are, so the ID needs to be derived from the partial flow graph.
There was a problem hiding this comment.
The UUID of Z/A would be dependent on the previous elements (e.g. either B or C) therefore, we can differentiate between flows to A from B and flows to A from C. Therefore the UUID of the vertices that reference A would be different.
For Z, it would take the previous vertex (so A) and its ID an generate a UUID from that. As the UUID of A is different in the diffrent cases, the UUID of Z would be different as well
There was a problem hiding this comment.
Or am I misunderstanding your point?
There was a problem hiding this comment.
Maybe my diagram wasn't clear
This would for example create 2 User vertices in two different partial tfg's. Their parameters would be equal
There was a problem hiding this comment.
This should not be fixed in 12601fe, as the UUID now depends on the ID of the following element as well (if it is present)
There was a problem hiding this comment.
The whole point of the UUID is to have a unique, not random ID for elements that stays the same during runs of the analysis.
Therefore, I would further revise the approach to include the ID of the outgoing Port as well.
In the Example, to get the UUID of the Function Node, it would first calculate the UUID of the previous vertex and pass its Node ID and the ID of the sending Port to it. Therefore, the UUIDs of the two Vertices that reference User would be different, as the sending port is different.
I know this solution is pretty complicated, but is still better that really not being able to reference a vertex between runs consistently.
There was a problem hiding this comment.
I've implemented this in 0d14062
There was a problem hiding this comment.
Nothing against creating the type 3 uuids from some meaningful properties from the nodes or something, but if we do not need those properties to be encoded there, I opt for using the random uuid functionality.
Looking into how EcoreUtils does this, they seem to mainly use the timestamp to generate a type 2 uuid. As huge as the type 4 UUIDs that are generated are, I do not see a problem.
This would also make the code simpler as it just needs to be implemented in AbstractVertex.
There was a problem hiding this comment.
pinDFDVertexMap only maps the input Pins to the previous vertices. A vertex holds no information about its successor. Therefore in this case Function would grap user to compute its ID. As Users pinDFDVertexMap is empty the user vertices would be indistinguishable.
If you don't want random ID's or to use the partial TFG they are residing in, you need to make the ID depended on the Ids of the incoming flows and then hand it to its predecessor. The incoming flow is the only part in which the vertices differ.
There was a problem hiding this comment.
The whole point is to have UUIDs that do not change between (test) runs.
But let's talk about this on Wednesday
| uuidString.append(previousElement.getUniqueIdentifier()); | ||
| } | ||
| return UUID.nameUUIDFromBytes(uuidString.toString().getBytes(StandardCharsets.UTF_8)); | ||
| } |
There was a problem hiding this comment.
pinDFDVertexMap only maps the input Pins to the previous vertices. A vertex holds no information about its successor. Therefore in this case Function would grap user to compute its ID. As Users pinDFDVertexMap is empty the user vertices would be indistinguishable.
If you don't want random ID's or to use the partial TFG they are residing in, you need to make the ID depended on the Ids of the incoming flows and then hand it to its predecessor. The incoming flow is the only part in which the vertices differ.
Entenwilli
force-pushed
the
unique-id-changes
branch
from
0d14062 to
4ed005e
Compare
This PR implements a unique ID for vertices in a transpose flow graph.
The UUID is unique on a vertex basis between all transpose flow graphs (e,g. there exists no vertex in all transpose flow graphs with the same UUID)
This PR closes #223