Skip to content

Avoid useless deepcopy of target with custom pulse gates in transpile #10973

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

Merged
merged 2 commits into from
Oct 5, 2023
Merged

Avoid useless deepcopy of target with custom pulse gates in transpile #10973

merged 2 commits into from
Oct 5, 2023

Conversation

itoko
Copy link
Contributor

@itoko itoko commented Oct 4, 2023

Summary

Improve transpilation time of circuits for a target with custom pulse gates by avoiding unnecessary deepcopy of the target object.
The time to build pulse schedules with circuit instructions is also improved since transpile function is internally called for each circuit instruction call.

Details and comments

When both inst_map and target are supplied, transpile function internally deepcopy the target object and update calibrations in it with ones in inst_map only if inst_map contains custom calibrations. However, the check was incorrect for the case when only the target (that contains custom calibrations) is supplied. In that case, the target object was deepcopied unnecessarily. This commit avoids it to improve the performance of transpilation with a custom target and hence building pulse schedules with circuit instructions. For instance, building 200 pulse schedules took 2 minutes while it takes 7 seconds with this commit.

@itoko itoko requested a review from a team as a code owner October 4, 2023 14:58
@qiskit-bot
Copy link
Collaborator

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@itoko itoko requested a review from nkanazawa1989 October 4, 2023 14:58
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, it's simple enough change but there is a lint failure

@mtreinish mtreinish added performance Changelog: Bugfix Include in the "Fixed" section of the changelog stable backport potential The bug might be minimal and/or import enough to be port to stable labels Oct 4, 2023
@mtreinish mtreinish added this to the 0.25.3 milestone Oct 4, 2023
@coveralls
Copy link

Pull Request Test Coverage Report for Build 6413465978

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 12 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.007%) to 87.019%

Files with Coverage Reduction New Missed Lines %
qiskit/extensions/quantum_initializer/squ.py 2 84.15%
crates/qasm2/src/lex.rs 4 91.16%
crates/qasm2/src/parse.rs 6 97.6%
Totals Coverage Status
Change from base Build 6407307416: 0.007%
Covered Lines: 74164
Relevant Lines: 85227

💛 - Coveralls

@mtreinish mtreinish added this pull request to the merge queue Oct 5, 2023
Merged via the queue into Qiskit:main with commit d0effae Oct 5, 2023
mergify bot pushed a commit that referenced this pull request Oct 5, 2023
…#10973)

* Avoid useless deepcopy of target in transpile

* Fix lint

(cherry picked from commit d0effae)
github-merge-queue bot pushed a commit that referenced this pull request Oct 5, 2023
…#10973) (#10978)

* Avoid useless deepcopy of target in transpile

* Fix lint

(cherry picked from commit d0effae)

Co-authored-by: Toshinari Itoko <15028342+itoko@users.noreply.github.com>
rupeshknn pushed a commit to rupeshknn/qiskit that referenced this pull request Oct 9, 2023
…Qiskit#10973)

* Avoid useless deepcopy of target in transpile

* Fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog performance stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants