Skip to content

Conversation

@daxfohl
Copy link
Collaborator

@daxfohl daxfohl commented Mar 17, 2022

Allows parameters for coefficient of PauliString and GlobalPhase:

  • Fix up the types.
  • Only cast to complex in initializer if it's numeric.
  • Add handlers for parameter resolution.
  • Raise errors in a couple functions that do non-basic math on coefficient (most functions just multiply, which should be fine).
  • Return False from has_unitary. (Side note, should we just add that check to the protocol method itself--we repeat it everywhere?)
  • Add a block for circuit_diagram_info when the coefficient is parameterized.
  • Add tests, update pydocs.

Also tests and fixes #4508 @tanujkhattar

@daxfohl daxfohl requested review from a team, cduck and vtomole as code owners March 17, 2022 01:01
@daxfohl daxfohl requested a review from tanujkhattar March 17, 2022 01:01
@CirqBot CirqBot added the size: M 50< lines changed <250 label Mar 17, 2022
@tanujkhattar
Copy link
Collaborator

@daxfohl Great timing! I added a comment / complaint in #5085 today that GlobalPhaseGate don't support symbols. Looks like they will soon. :))

@daxfohl
Copy link
Collaborator Author

daxfohl commented Mar 17, 2022

(Note that contrary to what I said the linked issue, decompose doesn't work when parameterized anyway since it's not unitary, so we actually didn't need to change GlobalPhaseGate here. But I'd already done it before I noticed it was unnecessary, so I figured I may as well leave the change in place since it seems non-invasive and probably useful.)

@daxfohl
Copy link
Collaborator Author

daxfohl commented Mar 17, 2022

Fixed diagonalgate while I was here. Required changing np.exp(1j * hat_angles[0])) to 1j ** (2 * hat_angles[0] / np.pi) because np.exp breaks on a symbol.

@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 18, 2022

@tanujkhattar do you mind taking this one?

@tanujkhattar tanujkhattar self-assigned this Apr 18, 2022
Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM % nits.

This is great, Thanks!

@daxfohl daxfohl requested a review from tanujkhattar April 21, 2022 04:12
Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot for the changes!

@tanujkhattar tanujkhattar added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Apr 22, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Apr 22, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Apr 22, 2022

Automerge cancelled: A required status check is not present.

Missing statuses: ['Misc check', 'Notebook formatting', 'Pytest MacOS (3.8)', 'Pytest MacOS (3.9)', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Type check', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage']

@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Apr 22, 2022
@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 22, 2022

@tanujkhattar Can you kick cirqbot again?

@tanujkhattar tanujkhattar merged commit 839ebb1 into quantumlib:master Apr 23, 2022
@daxfohl daxfohl deleted the pauliparam branch April 23, 2022 05:24
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M 50< lines changed <250

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parameterized DensePauliString (a gate) cannot be applied on qubits to produce an operation

4 participants