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, BarrierBeforeFinalMeasurements, DAGFixedPoint and Decompose transpiler passes, fixes issue #7485 #7989

Closed
wants to merge 5 commits into from

Conversation

TheGupta2012
Copy link
Contributor

@TheGupta2012 TheGupta2012 commented Apr 27, 2022

Summary

Fix memory usage issues in BasisTranslator, DAGFixedPoint and Decompose transpiler passes, fixes #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.

@TheGupta2012 TheGupta2012 requested a review from a team as a code owner April 27, 2022 11:38
@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
Copy link
Contributor Author

This a copy of the PR 7978, made from my local fork, as the organizational level forks did not allow edits by maintainers.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 2232806192

  • 15 of 15 (100.0%) changed or added relevant lines in 3 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.002%) to 84.325%

Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 2231814429: -0.002%
Covered Lines: 54527
Relevant Lines: 64663

💛 - Coveralls

@TheGupta2012 TheGupta2012 changed the title [WIP] Fix memory usage issues in BasisTranslator, DAGFixedPoint and Decompose transpiler passes, fixes issue #7485 [WIP] Fix memory usage issues in BasisTranslator, BarrierBeforeFinalMeasurements, DAGFixedPoint and Decompose transpiler passes, fixes issue #7485 Apr 27, 2022
@HuangJunye HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 21, 2022
@1ucian0
Copy link
Member

1ucian0 commented Apr 25, 2023

Hi @TheGupta2012 ! Do you mind showing the memory usage improvement like in #7978 (comment) ?

@jakelishman
Copy link
Member

Closing this as stale / no response now, but please feel free to re-open if that's not the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community PR PRs from contributors that are not 'members' of the Qiskit repo
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

BasisTranslator, Decompose, BarrierBeforeFinalMeasurements, DAGFixedPoint using a lot of memory
6 participants