Skip to content

Simplify decomposition of controlled eigengates with global phase #7238

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
Open
daxfohl opened this issue Apr 8, 2025 · 9 comments · May be fixed by #7291
Open

Simplify decomposition of controlled eigengates with global phase #7238

daxfohl opened this issue Apr 8, 2025 · 9 comments · May be fixed by #7291
Labels
area/decomposition area/eigengate good for learning For beginners in QC, this will help picking up some knowledge. Bit harder than "good first issues" kind/design-issue A conversation around design status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@daxfohl
Copy link
Collaborator

daxfohl commented Apr 8, 2025 •
edited
Loading

Describe your design idea/issue

The gate cirq.XPowGate(global_shift=0.22).controlled() decomposes to

Z**-0.28(q(0))
Z**0.5000000000000002(q(1))
(0.7071067811865474-0.7071067811865477j)
Y**-0.5(q(1))
CZ(q(0), q(1))
Y**0.5(q(1))
Z**-0.21999999999999992(q(1))
X**0.5(q(1))
S(q(1))
(0.33873792024529137-0.9408807689542256j)
Y**-0.5(q(1))
CZ(q(0), q(1))
Y**0.5(q(1))
Y**-0.49999999999999994(q(1))
Z**1.2199999999999998(q(1))
(-0.42577929156507277+0.9048270524660196j)

But it's easy to see that cirq.XPowGate(global_shift=0.22).controlled() can be broken down into a regular X gate plus a GlobalPhaseGate(0.22), with the same control applied to each. The controlled X is therefore just CX, which has a defined decomposition to [Y, CZ, Y], and a controlled phase gate is equivalent to a Z**phase. Therefore, cirq.XPowGate(global_shift=0.22).controlled() is logically equivalent to and probably should decompose to

Y**-0.5(q1)
CZ(q0, q1)
Y**0.5(q1)
Z**0.22(q0)

This similarly happens with other controlled instances of EigenGate with global phases, where ControlledGate._decompose_ sends them through the raw numeric matrix reduction transformers, instead of just factoring out the phase first.

An approach to solve this could be

  1. Add a default EigenGate._decompose_ that factors out any phase into its own gate. i.e. it returns a phase-free EigenGate and a separate global phase gate.
  2. In ControlledGate._decompose_, move the matrix transformer option to be the last option to try (after attempting the decomposition of the subgate). This would allow the controlled phased gate to decompose into a controlled unphased gate plus a controlled global phase, which generally will be simpler to decompose further from there.
@daxfohl daxfohl added kind/design-issue A conversation around design good part-time project A meaty non-urgent issue with a substantial amount of work to be done. good for learning For beginners in QC, this will help picking up some knowledge. Bit harder than "good first issues" area/eigengate area/decomposition labels Apr 8, 2025
@daxfohl daxfohl changed the title Improve decomposition of controlled eigengate with global phase Simplify decomposition of controlled eigengates with global phase Apr 8, 2025
@daxfohl daxfohl mentioned this issue Apr 8, 2025
Open
@daxfohl daxfohl removed the good part-time project A meaty non-urgent issue with a substantial amount of work to be done. label Apr 10, 2025
@mhucka mhucka added the triage/discuss Needs decision / discussion, bring these up during Cirq Cynque label Apr 16, 2025
@daxfohl daxfohl mentioned this issue Apr 16, 2025
Open
@codrut3
Copy link
Contributor

codrut3 commented Apr 19, 2025

I spent some time looking into this and I have several questions:

  • what if a gate derives from EigenGate and fixes the global phase? I found that Rx, Ry, Rz and MSGate set global_shift=-0.5. There can be more gates like this that I am not aware of.
  • what if the global phase or the exponent are parametrized, should the phase still be put in a global phase gate? Then the global phase gate will be parametrized, and ControlledGate._decompose_ returns NotImplemented for parametrized global phase gates.

@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 20, 2025 •
edited
Loading

Your other PR should mitigate the second concern. It'll decompose to its canonical form, which is a Z with parameterized exponent. The first concern might be a blocker though. It definitely doesn't make sense to factor out the phase of a rotation gate. Open to ideas.

@codrut3
Copy link
Contributor

codrut3 commented Apr 20, 2025

I found that Rx, Ry and Rz are endpoints for cirq.decompose. Then I can define _decompose_ for each of them and make it return NotImplemented. This stops the decomposition at the gate, just as it happens right now.
Luckily, the superclass of MSGate defines _decompose_, so it's not affected by the introduction of EigenGate._decompose_.
So the PR looks doable, provided I didn't miss any gate that fixes global_shift.

@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 21, 2025

Yeah I guess it's either opt-out or opt-in, but each subclass that opts will have to specify that itself. I can't think of a clever way to get around it, or at least not a good one.

@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 21, 2025

Initial thoughts:

  1. It's probably safer to go with the opt-in approach, for backwards compatibility. If anyone has defined their own Rx equivalent, then we don't want to break them.
  2. Even further, we probably only want to do this when decomposing a controlled gate. Right now, X, Y, Z don't decompose in any case so it would be strange if they started to.

I think both problems are solvable. For 1, EigenGate could define a default _allow_phase_extraction_in_decompose = False, which could be overridden in the desired subgates, and EigenGate._decompose_ could check that and return the unphased self plus a global phase if that flag is set. For 2, the decompose context could have a new member extract_phases = False. EigenGate._decompose_ would have to check that flag too, and ControlledGate._decompose_, would have to set that flag to True when decomposing the subgate. This approach would also allow users to extract phase when decomposing non-controlled gates too if they wanted.

@codrut3 codrut3 linked a pull request Apr 21, 2025 that will close this issue
Open
@codrut3
Copy link
Contributor

codrut3 commented Apr 21, 2025

Thank you for the suggestions! Please see #7291 for my attempt to address this issue.

I think there is no nice solution here, so I am fine if the PR is not accepted.

@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 21, 2025

I think it's a good start. This assumes that the issue itself gets approved. I think primarily see if you can get rid of the type check. If it's not possible, I think that indicates this may be a step in the wrong direction anyway. If it's possible, then that adds some weight to this being a worthwhile change.

@daxfohl
Copy link
Collaborator Author

daxfohl commented Apr 26, 2025 •
edited
Loading

Oh, backing up a little, I found the documentation here states that the decomposition should terminate in a Pauli, CZ, or global phase.

https://github.com/quantumlib/Cirq/blob/main/cirq-core/cirq/protocols/decompose_protocol.py#L98

Given that, it seems like the best approach here would be to instrument the Paulis and CZ each with a _decompose_ that extracts the global phase, and follow the "For 2" guideline in my comment above for backward compatibility. This approach would avoid breaking fixed-phase EigenGates and would avoid any changes to non-controlled gates (unless the user explicitly sets extract_phases=True), but still provide the benefit of simpler decompositions of controlled EigenGates, including ones mentioned in the original issue, and controlled rotation gates since they inherit from the paulis.

A shared EigenGate._extract_phase that returns the unphased original gate and optional global phase could be used to reduce code duplication in the different _decompose_ methods.

@mhucka mhucka added triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation and removed triage/discuss Needs decision / discussion, bring these up during Cirq Cynque labels Apr 30, 2025
@mhucka
Copy link
Contributor

mhucka commented Apr 30, 2025

Discussed in Cirq Cynq 2025-04-30: accepting the issue. Needs more investigation of the operations involved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/decomposition area/eigengate good for learning For beginners in QC, this will help picking up some knowledge. Bit harder than "good first issues" kind/design-issue A conversation around design status/needs-agreed-design We want to do this, but it needs an agreed upon design before implementation triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Simplify decomposition of controlled eigengates with global phase codrut3/Cirq
3 participants
@daxfohl @mhucka @codrut3