Issue when cloning MetadataStore

[AyoubMDL]

Hello,

When cloning a model whose nodes contain a MetadataStore, the underlying metadata objects are shallow copied. Therefore, the original and cloned models share references to the same metadata entries (see code below).

import onnx_ir as ir

def check_meta():
    ir_model = ir.from_onnx_text("""
                < ir_version: 10, opset_import: ["" : 21] >
                test_model (float[N, 32] X) => (float [N, ?] Y)
                {
                    Y = Relu(X)
                }
            """)

    # Add metadata to the node
    ir_model.graph[0].meta["test_meta"] = "test_value"

    # Clone the model
    cloned_ir_model = ir_model.clone()

    # Check that the metadata is cloned
    assert cloned_ir_model.graph[0].meta is not ir_model.graph[0].meta

    # Check that the metadata value is the same
    print(
        cloned_ir_model.graph[0].meta["test_meta"] is ir_model.graph[0].meta["test_meta"]
    )  # -> True

check_meta()

The fix is that MetadataStore content should be deep copied. I can open a PR if necessary.

[justinchuby]

The shallow copy is intentional, because we don't really know if the metadata entries can be deep copied. Can you share your use case? Maybe you can help adding an option to the clone() method to optionally do a deep copy?

[AyoubMDL]

Can you share your use case?

It's actually a very specific scenario. I was writing unit tests to verify some transformation behavior https://github.com/AyoubMDL/onnx_quantize/blob/9fc6062ab69fdf5b5d7bcc8ffa6c55e64a6dbd47/test/pre_passes/test_smooth_quant.py#L131C5-L135C6 , I start with an ir model whose nodes have meta["input"]. I clone this model and apply some transforms to the cloned model's meta so I can compare the results later. However, the tests kept failing because modifying the cloned model's meta was also changing the original model’s meta.

we don't really know if the metadata entries can be deep copied

Is it not possible to just use copy.deep_copy since these are Python objects?

[justinchuby]

deep_copy is not always reliable, I think, since we can expect the metadata containing graph like objects (which can have cyclic pointers). Also the complexity of deep copy is unpredictable. It would be good for clone() to be generally fast, by default. I think a deep copy mode for clone can make sense.

Also in your use case, what is meta["input"]? Is it an ir.Value?

[AyoubMDL]

Also in your use case, what is meta["input"]? Is it an ir.Value?

It's a numpy array. Also, I use meta to store other things during the process but with no cloning (expect in this unit test)