[DRAFT] Merge metadata props when fusing #2375
Conversation
bmehta001
force-pushed
the
merge-docstrings-when-fusing
branch
from
403c9f3 to
0ffe2bd
Compare
bmehta001
force-pushed
the
merge-docstrings-when-fusing
branch
from
0ffe2bd to
cd8ea57
Compare
| fused_metadata_props = "Fused: " + "\t\n".join( | ||
| n.metadata_props for n in delta.match.nodes if getattr(n, "metadata_props", None) | ||
| ) |
There was a problem hiding this comment.
n will always have metadata_props.
| n.metadata_props for n in delta.match.nodes if getattr(n, "metadata_props", None) | ||
| ) | ||
| # Assign to all new nodes (or just the first, depending on your policy) | ||
| delta.new_nodes[0].metadata_props += fused_metadata_props |
There was a problem hiding this comment.
Prefer metadata_props.update()
| # If we are fusing nodes, update the metadata props of the new node(s) | ||
| if delta.match.nodes and delta.new_nodes: | ||
| # Concatenate metadata props from all original nodes | ||
| fused_metadata_props = "Fused: " + "\t\n".join( |
There was a problem hiding this comment.
I would consider creating special reconciliation logic for known fields
There was a problem hiding this comment.
+1. I am also confused a bit here. Isn't metadata_props effectively a dict[str, str]? I am not sure how the proto is represented in onnx-ir, but assuming it is a dictionary. We should be merging the dictionaries, using special merging logic for known entries in the dictionary.
This may require us to extend the definition/documentation of specific metadata fields. For example, we may need a way to represent a set of source-locations, instead of a single source-location, for a source-location metadata.
No description provided.