Make a shallow copy of metadata in QuantumCircuit <> DAG conversion#16142
Make a shallow copy of metadata in QuantumCircuit <> DAG conversion#16142matteoacrossi wants to merge 1 commit intoQiskit:mainfrom
metadata in QuantumCircuit <> DAG conversion#16142Conversation
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the following people are relevant to this code:
|
jakelishman
left a comment
There was a problem hiding this comment.
Thanks for the PR. I left a few comments about why I'm not sure we can unilaterally make this change in #16140 (comment) - I think we're going to need to take a bit more care with API stability, because people could easily be relying on the current behaviour.
It's probably best to keep the conversation over there for the time being, then pivot this PR?
| m.call_method0(intern!(py, "copy")).ok() | ||
| } | ||
| }), | ||
| transpile_layout: ob.getattr(intern!(py, "layout")).ok(), |
There was a problem hiding this comment.
This place (Rust-space extraction of the fields from a Python-space object) is most likely not the right place to put this - that's going to cause the copy in the wrong places, I think. It so happens that circuit_to_dag calls this right now (I think?), but that's just one of the users.
Fixes #10570. Fixes #16140.
As discussed in #10570, metadata is only assigned in
circuit_to_daganddag_to_circuit, resulting in potential side effects when modifying the metadata of one of the circuit instances.This PR implements the change proposed in #10570 to
dag_to_circuit, making a shallow copy of themetadatafield.For
circuit_to_dagthe change is made inconverters.rs, I hope it is the right place.It also adds two quick tests.
AI/LLM disclosure