Skip to content

Avoid direct-CXs in RZX calibration pass #8276

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Jul 4, 2022
Merged

Avoid direct-CXs in RZX calibration pass #8276

merged 6 commits into from
Jul 4, 2022

Conversation

nkanazawa1989
Copy link
Contributor

@nkanazawa1989 nkanazawa1989 commented Jun 29, 2022 •
edited
Loading

Summary

RZX calibration pass is implemented for CX with echoed cross resonance sequence. Recent IBM backends started to support another realization of CX with so called direct-CX. Generation of fractional gate for this calibration is not straightforward because dynamics on the target qubit is much more complicated.

In this PR, passes are updated to avoid termination by errors. This is temporary solution, just not to crash the transpiler. We need some additional research for the technique to stretch the direct-CX pulses. It is expected that these passes are upgraded again with support for direct-CXs.

Details and comments

In addition to error-avoid mechanism, some cleanup has done in this PR. The logic for error avoidance is very simple. I newly introduced CalibrationNotAvailable exception, which is raised when the pass encounters the direct-CX. This is caught by the base class CalibrationBuilder to ignore invalid calibrations.

@nkanazawa1989 nkanazawa1989 requested review from a team, eggerdj and DanPuzzuoli as code owners June 29, 2022 20:01
@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

@coveralls
Copy link

coveralls commented Jun 29, 2022 •
edited
Loading

Pull Request Test Coverage Report for Build 2611047339

  • 149 of 194 (76.8%) changed or added relevant lines in 6 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 83.999%
Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/calibration/rzx_builder.py 101 146 69.18%
Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 89.36%
Totals Coverage Status
Change from base Build 2611045085: 0.03%
Covered Lines: 55820
Relevant Lines: 66453
💛 - Coveralls

@nkanazawa1989
Step1. Add handling for direct CX in RZX calibration builder. PulseEr…
b989392 
…ror is replaced with User warning not to crash pass manager execution.
@nkanazawa1989
Step2. Cleanup RZX builder. Unreachable errors are removed. Use inpla…
2bb8e11 
…ce mode for schedule construction to avoid redundant deep copy. Deprecate unused argument.
@nkanazawa1989
Step3. Reorganize calibration module structure. Usually, in the trans…
c0f0391 
…pile pass modules, single file contains a single (or similar) pass. RZX builder passes are moved to rzx_builder, Pulse gate pass is moved to pulse_gate and base class is moved to base_builder
@nkanazawa1989
Step4. Write release note
60d85d3
@nkanazawa1989
Copy link
Contributor Author

nkanazawa1989 commented Jun 30, 2022 •
edited
Loading

@taalexander I split commit into multiple pieces.

  1. b989392
    This is the minimum change to avoid pass manager crash. Test is also added.

  2. 2bb8e11
    This is cleanup of RZX builder implementation. Basically, this commit does

  • Deprecate unused argument.
  • Redesign the logic to remove unreachable errors.
  • Use inline mode to build schedule to avoid redundant deep copy.

  1. c0f0391
    This is for reorganization of the module. Currently several passes are packed together into a single file, but this is discouraged in other pass modules. (code is not changed!)

  2. 60d85d3
    This is a commit for release note.

taalexander
taalexander requested changes Jun 30, 2022
View reviewed changes
Copy link
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor comments.

@nkanazawa1989 @taalexander
docs update and turn a cal validation into a separate function. calib…
24d8dcb 
…ration type enum is added for user-friendly error message.

Co-authored-by: Thomas Alexander <talexander@ibm.com>
@mergify mergify bot merged commit 36c91ad into Qiskit:main Jul 4, 2022
@wshanks wshanks mentioned this pull request Sep 9, 2022
Closed
@nkanazawa1989 nkanazawa1989 deleted the fix/rzx_pass_ddcx branch November 25, 2022 02:55
@1ucian0 1ucian0 mentioned this pull request Oct 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@taalexander taalexander taalexander approved these changes

@eggerdj eggerdj Awaiting requested review from eggerdj

@DanPuzzuoli DanPuzzuoli Awaiting requested review from DanPuzzuoli

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nkanazawa1989 @qiskit-bot @coveralls @taalexander