Add Code to Preserve Metadata During Transformation #211
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maltanar
requested changes
Nov 5, 2025
There was a problem hiding this comment.
Since it's hard to reason about how metadata should behave when nodes are created/deleted it will be tricky to test this in general, but we can approach this as a general infrastructure feature. A few questions and requests in addition:
- Is it only node metadata you care about? What about tensor metadata when tensors get created or erased? (e.g. FINN makes use of tensor metadata to indicate inferred datatypes, structured sparsity when lowering depthwise convolutions)
- How should
copy_metadata_propsbehave if the same named attribute exists in both source node and destination node (or multiple source nodes)? Can you add this in a unit test? - For the metadata use-cases you can think of for particular transformations, please add a few unit tests that check how the metadata is expected to behave.
- Why are some transformations left out, while others are included? e.g.
BatchNormToAffinealso creates new nodes butcopy_metadata_propsis not added there.
Comment on lines
+355
to
+384
| def copy_metadata_props(source_node, target_node): | ||
| """Copy metadata properties from source node(s) to target node. | ||
| Parameters | ||
| ---------- | ||
| source_node : onnx.NodeProto or list of onnx.NodeProto | ||
| Source node(s) from which to copy metadata_props. If a list is provided, | ||
| metadata from all nodes will be merged into the target node. | ||
| target_node : onnx.NodeProto | ||
| Target node to which metadata_props will be copied. | ||
| Returns | ||
| ------- | ||
| None | ||
| Modifies target_node in place by extending its metadata_props. | ||
| Examples | ||
| -------- | ||
| >>> # Copy from single node | ||
| >>> copy_metadata_props(old_node, new_node) | ||
| >>> | ||
| >>> # Copy from multiple nodes (e.g., when fusing) | ||
| >>> copy_metadata_props([quant_node, dequant_node], fused_node) | ||
| """ | ||
| # Handle both single node and list of nodes | ||
| source_nodes = source_node if isinstance(source_node, list) else [source_node] | ||
|
|
||
| for node in source_nodes: | ||
| if hasattr(node, "metadata_props"): | ||
| target_node.metadata_props.extend(node.metadata_props) |
There was a problem hiding this comment.
Please provide some unit test(s) for this new utility function, ideally also covering edge cases (e.g. source node has no attributes, source and target node have attribute with the same name).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates qonnx with metadata preservation during transformation. This operates on the following rules: