Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deepcopies from DAGCircuit.__eq__ #4992

Merged
merged 2 commits into from
Sep 18, 2020

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Aug 28, 2020

Summary

The __eq__ method of the dagcircuit class was previously deep copying
the retworkx PyDAG objects for both self and other to force a reindexing
of the node indices in the graphs to acvoid a bug in retworkx (see
Qiskit/rustworkx#27 for more details). In the next retworkx a workaround
for that bug was introduced which removes the need for doing the deepcopy
calls. This commit does that and removes the deep copies which should
improve the performance of run __eq__ on dagcircuit objects.

Details and comments

The __eq__ method of the dagcircuit class was previously deep copying
the retworkx PyDAG objects for both self and other to force a reindexing
of the node indices in the graphs to acvoid a bug in retworkx (see
Qiskit/rustworkx#27 for more details). In the next retworkx a workaround
for that bug was introduced which removes the need for doing the deepcopy
calls. This commit does that and removes the deep copies which should
improve the performance of run __eq__ o dagcircuit objects.
@mtreinish mtreinish added on hold Can not fix yet performance labels Aug 28, 2020
@mtreinish mtreinish requested a review from a team as a code owner August 28, 2020 11:51
@mtreinish
Copy link
Member Author

Marking as on hold because retworkx 0.5.0 isn't ready yet

@mtreinish mtreinish added this to the 0.16 milestone Sep 18, 2020
@mtreinish mtreinish removed the on hold Can not fix yet label Sep 18, 2020
@mtreinish
Copy link
Member Author

I released retworkx 0.5.0 a few minutes ago so this is no longer blocked

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants