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

Pauli-X measurement instruction #7716

Closed
wants to merge 4 commits into from
Closed

Pauli-X measurement instruction #7716

wants to merge 4 commits into from

Conversation

yaelbh
Copy link
Contributor

@yaelbh yaelbh commented Feb 28, 2022

Summary

This PR replaces the old PR #5311, while addressing the comments there.

Details and comments

  1. This is only measure_x, when it's finalized (after review) I'll write measure_y.
  2. Afterwards we can consider @itoko's suggestion Pauli measurement #5311 (comment).

Ready for review if the CI passes.
@1ucian0 @nkanazawa1989 @itoko

@coveralls
Copy link

coveralls commented Feb 28, 2022

Pull Request Test Coverage Report for Build 1915762000

  • 44 of 56 (78.57%) changed or added relevant lines in 8 files are covered.
  • 18 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.01%) to 83.451%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/visualization/text.py 14 15 93.33%
qiskit/circuit/measure_x.py 20 24 83.33%
qiskit/visualization/matplotlib.py 2 9 22.22%
Files with Coverage Reduction New Missed Lines %
qiskit/transpiler/passes/routing/stochastic_swap.py 5 96.18%
qiskit/init.py 13 67.19%
Totals Coverage Status
Change from base Build 1904273099: 0.01%
Covered Lines: 52444
Relevant Lines: 62844

💛 - Coveralls

Copy link
Contributor

@nkanazawa1989 nkanazawa1989 left a comment

Choose a reason for hiding this comment

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

LGTM. I like this implementation in calibration perspective. Since we can attach calibration per every instruction, a general Pauli measure instruction is difficult to manage because schedule may change depends on the basis.

(but I will hold my approval since I'm not expert of circuit instruction)

@1ucian0 1ucian0 self-assigned this Mar 2, 2022
@1ucian0 1ucian0 mentioned this pull request Mar 2, 2022
3 tasks
@ikkoham
Copy link
Contributor

ikkoham commented Mar 8, 2022

I have a question. Should QC.remove_final_measurements remove this instruction or not?

@yaelbh
Copy link
Contributor Author

yaelbh commented Mar 8, 2022

@ikkoham This is a good question. See also here: #5311 (comment).

@1ucian0
Copy link
Member

1ucian0 commented Mar 17, 2022

I have a question. Should QC.remove_final_measurements remove this instruction or not?

I think RemoveFinalMeasurements should remove this measurement. The original motivation was to create a statevector out of the circuit #2903

@ikkoham
Copy link
Contributor

ikkoham commented Mar 18, 2022

Thank you, it would be useful to have an option to leave the Hadamard gate when removing the X measurement.

@ajavadia
Copy link
Member

Since we can attach calibration per every instruction, a general Pauli measure instruction is difficult to manage because schedule may change depends on the basis.

What do you mean by this @nkanazawa1989 ?

Also is the experiments usecase here that there is a separate measure_x calibration that is different from just a Hadamard+measure?

@yaelbh
Copy link
Contributor Author

yaelbh commented May 2, 2022

The purpose of this PR is to speed-up Aer (statevector method) for VQE circuits. The idea is that VQE circuits will express their expectation value using the new instructions. Consequently, Aer will be able to efficiently simulate multi-qubit Pauli measurements.

I'm not sure if the last couple of sentences make sense; at least they made sense some time ago, when I started this work. Possibly, since then, the interface between VQE and Aer has changed, especially in light of the new Estimator primitive.

I'm closing this PR, because currently I'm not focused on classical simulation for VQE. I find the addition of instructions less simple than it looks, for reasons detailed in previous comments. Back then, we (@chriseclectic and I) considered two alternatives. The one that we decided to pursue is to add instructions to Terra, and let Aer benefit from them. The other alternative is to build a mechanism in Aer that detects Pauli measurement instructions, in circuits that don't contain them explicitly.

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.

6 participants