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

[WIP] Fix memory usage issues in BasisTranslator, DAGFixedPoint and Decompose transpiler passes, fixes issue #7485 #7978

Conversation

TheGupta2012
Copy link
Contributor

@TheGupta2012 TheGupta2012 commented Apr 22, 2022

Summary

Fix memory usage issues in BasisTranslator, DAGFixedPoint and Decompose transpiler passes, fixes issue #7485

Details and comments

Decompose

  • The original _should_decompose method tried to make two variables strings_list and gate_type_list repeatedly during the call of the method.
  • This should not be the case as the lists only depend on the gates_to_decompose attribute of the pass, which is invariant during the call of the run method
  • New attributes are introduced for the strings_list and the gate_type_list lists.
  • Although this may not have much effect on the memory usage of the pass, this should have some improvement in the run time of the pass.

BasisTranslator - to do

BarrierBeforeFinalMeasurements

  • There is a dependency of MergeAdjacentBarriers in the above pass.
  • The variable current_barrier_nodes here was used only for membership checking
  • A set would serve as a more optimal data structure here
  • Note : Memory leakage is yet to be detected

DAGFixedPoint

  • There was a usage of deepcopy, for saving a copy of the dag, in the property set of the circuit.
  • This may have been a point of memory leakage and thus an explicit delete was introduced before the re-assignment.

@qiskit-bot
Copy link
Collaborator

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 the following people are requested to review this:

  • @Qiskit/terra-core

@TheGupta2012 TheGupta2012 changed the title Fix memory usage issues in BasisTranslator, DAGFixedPoint and Decompose transpiler passes, fixes issue #7485 [WIP] Fix memory usage issues in BasisTranslator, DAGFixedPoint and Decompose transpiler passes, fixes issue #7485 Apr 22, 2022
@coveralls
Copy link

coveralls commented Apr 22, 2022

Pull Request Test Coverage Report for Build 2228333600

  • 15 of 15 (100.0%) changed or added relevant lines in 3 files are covered.
  • 22 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+0.02%) to 84.264%

Files with Coverage Reduction New Missed Lines %
qiskit/circuit/controlflow/control_flow.py 1 83.33%
qiskit/circuit/controlflow/while_loop.py 1 97.96%
qiskit/transpiler/passes/optimization/optimize_1q_commutation.py 1 99.09%
qiskit/extensions/unitary.py 4 96.12%
qiskit/circuit/controlflow/if_else.py 7 96.77%
qiskit/transpiler/passes/basis/unroller.py 8 85.96%
Totals Coverage Status
Change from base Build 2208196800: 0.02%
Covered Lines: 54180
Relevant Lines: 64298

💛 - Coveralls

@TheGupta2012 TheGupta2012 marked this pull request as ready for review April 25, 2022 17:29
@TheGupta2012 TheGupta2012 requested a review from a team as a code owner April 25, 2022 17:29
@ewinston
Copy link
Contributor

I tested 08a2b0d and didn't appear to see a significant change in the memory usage.

image
image

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

Successfully merging this pull request may close these issues.

4 participants