merge_single_qubit_gates_to_phxz shouldn't return ops.I

[NoureldinYosri]

return PhasedXZGate(0, 0, 0) instead of I since callers expect the returned gates to be of type PhasedXZGate

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.59%. Comparing base (43f6849) to head (602016e).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8319      +/-   ##
==========================================
- Coverage   99.59%   99.59%   -0.01%     
==========================================
  Files        1125     1125              
  Lines      103250   103250              
==========================================
- Hits       102829   102828       -1     
- Misses        421      422       +1     

☔ View full report in Codecov by Harness.
📢 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.

[arettig]

Adding nonfunctional phxz gates could lead to slightly less optimized circuits when running this transformer (of course returning ops.I is also not ideal).

For your use case, do you need a gate at all? The best solution is probably to just return [] if gate and merge_tags_fn are both None. (I think this would handle the parameter sweep use case that caused the ops.I to be added in the first place)

[NoureldinYosri]

returning [] is also not ideal since that will change the structure of the circuit .. take for example a parameterized circuit + sweep if we do the [] thing then there is a chance that one of the gates gets dropped making that circuit have a different structure from the rest

[NoureldinYosri]

maybe an extra param is the answer?

[arettig]

maybe an extra param is the answer?

Yeah, that seems like the best option since we want both behaviors.

[pavoljuhas]

... there is a chance that one of the gates gets dropped making that circuit have a different structure from the rest

Is the concern that such circuit would have a different length? If so, would it help to produce an empty Moment() instead of identity-like gate?

[arettig]

Is the concern that such circuit would have a different length? If so, would it help to produce an empty Moment() instead of identity-like gate?

I think this would still not work. Even if the number of moments is the same, if we drop parameterized operations, then we could end up with incorrect symbolized circuits output from the merge_single_qubit_gates_to_phxz_symbolized transformer.

I was thinking we could use the existing merge_tags_fn parameter which is seemingly only used by the merge_single_qubit_gates_to_phxz_symbolized transformer and gives a tag if the merged operation is parameterized. Something like:

        gate = single_qubit_decompositions.single_qubit_matrix_to_phxz(u, atol)
        if not gate:
            if not merge_tags_fn or not merge_tags_fn(circuit_op):
                return []
            gate = ops.PhasedXZGate(axis_phase_exponent=0, x_exponent=0.0, z_exponent=0.0)

This would return a PhasedXZGate(0,0,0) for parameter sweeps run via merge_single_qubit_gates_to_phxz_symbolized but would remove identity operations otherwise.