Attempt canonicalization first when decomposing controlled gates #7242

[codrut3]

This fixes #7242

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.67%. Comparing base (65a4105) to head (8e38581).
Report is 17 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7269      +/-   ##
==========================================
+ Coverage   98.64%   98.67%   +0.02%     
==========================================
  Files        1106     1106              
  Lines       95985    96102     +117     
==========================================
+ Hits        94688    94824     +136     
+ Misses       1297     1278      -19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[daxfohl]

Is this condition possible? The constructor for ControlledGate absorbs the control layers if the subgate is another ControlledGate. Do any of the unit tests fail if this check is removed?

[codrut3]

Yes, I tried first without and the unit tests were failing.

I discovered that a 2-cycle forms in the decomposition for something like CCZ and control_qid_shape = [3].
The following happens:

• when the sub gate is CZ, CZPowGate.controlled() sees that control_qid_shape[-1] != 2 and returns a ControlledGate. Then self != controlled_sub_gate is false. Next, the special case in ControlledGate._decompose_with_context_() for CZPowGate creates a Z gate and adds +1 to the control qubits;
• when the sub gate is Z, the ZPowGate.controlled() method returns a CZPowGate and subtracts -1 from the control qubits. Then self != controlled_sub_gate is true.

This cycles infinitely, adding and removing a control qubit.

[daxfohl]

Do any of the unit tests depend on this condition? If #7241 is done, does that mitigate the need for this check?

[codrut3]

I am not sure, can you check the example in the comment above and see if it would work when #7241 is resolved?

[daxfohl]

Will give it a look tonight if I get a chance

[daxfohl]

Yeah, seems like if we make the end of CXPowGate.controlled look something like

        if self._global_shift != 0 or not isinstance(result, controlled_gate.ControlledGate):
            return result
        if (
            isinstance(result.control_values, cv.ProductOfSums)
            and result.control_values[-1] == (1,)
            and result.control_qid_shape[-1] == 2
        ):
            return cirq.CCXPowGate(
                exponent=self._exponent, global_shift=self._global_shift
            ).controlled(
                result.num_controls() - 1, result.control_values[:-1], result.control_qid_shape[:-1]
            )
        return controlled_gate.ControlledGate(
            XPowGate(exponent=self.exponent),
            num_controls=result.num_controls() + 1,
            control_values=result.control_values & cv.ProductOfSums([1]),
            control_qid_shape=result.control_qid_shape + (2,),
        )

(basically updating the final return value to be a controlled X with an extra control value, instead of a controlled CX), and similar for CZPowGate, then that mitigates the need for either of those checks.

I think any of the following approaches would work:

1. Do Eliminate multiple control layers on CX/CZ.controlled([0]) #7241 first and pull that into here once it's been merged.
2. Do Eliminate multiple control layers on CX/CZ.controlled([0]) #7241 and Attempt canonicalization first when decomposing controlled gates #7242 both here in the same PR.
3. Just add a comment to your if conditions that they can be removed once Eliminate multiple control layers on CX/CZ.controlled([0]) #7241 is complete.

[codrut3]

I would prefer to add a comment and then remove the check when doing #7241, what do you think?

[daxfohl]

That works. An ulterior motive for the set of issues I posted recently is to remove all the casting and type checking from ControlledGate.decompose. Once all four are complete, I'm pretty sure all of the type checks can be eliminated from that function, and things will generally be more consistent.

[codrut3]

Thank you! I added a comment.

[daxfohl]

lgtm cc @pavoljuhas