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

Update pass_operations_over with fixed conjugated_by #7123

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

babacry
Copy link
Collaborator

@babacry babacry commented Mar 5, 2025

Context1: pass_operations_over is supposed to be replaced by conjugated_by as being described in #2351. Unless it is widely used, we should deprecate this function as the order of the CliffordOps input is very confusing (like being suggested in #2351).

Context2 (existing implementations failed in a couple of cases, including some test cases): current implementation of pass_operations_over might crash like #6946. And it produced wrong results for e.g., (Z(q0)*Y(q1)).pass_operations_over([PauliInteractionGate(Z, True, X, True)(q0, q1)])=Y(q1), but should be -Y(q1).

Followup is to start deprecating pass_operations_over. (I choose to not start deprecating the function as I want to make sure all the pass_operations_over tests would pass under the new implementation, a separate pr to isolate the deprecation process would be cleaner given that there are dependencies in the code base)

  • Fix previous tests of pass_operation_over, 24 test cases in the previous unit test doesn't preserve unitaries using previous implementations of pass_operation_over. The CliffordOps in those failed test cases are constructed from PauliInteractionGate with different parameters. Updated the test so unitaries are preserved.

  • Also fixed a bug in conjugated_by.

  • Nit fixes to pauli_string.py. e.g., remove import cirq.

@CirqBot CirqBot added the size: M 50< lines changed <250 label Mar 5, 2025
@babacry babacry force-pushed the conj branch 2 times, most recently from a21cd5a to 4292cea Compare March 5, 2025 07:16
@babacry babacry self-assigned this Mar 5, 2025
@babacry babacry changed the title Update pass_operations_over with corrected conjugated_by. Update pass_operations_over with fixed conjugated_by. Mar 5, 2025
_assert_pass_over([op0], ps_before, ps_after)

op0 = cirq.PauliInteractionGate(Y, t_or_f1, X, t_or_f2)(q0, q1)
ps_before = cirq.PauliString({q0: Z, q2: Y}, sign)
ps_after = cirq.PauliString({q0: Z, q2: Y, q1: X}, sign)
ps_after = cirq.PauliString({q0: Z, q2: Y, q1: X}, -sign if t_or_f2 else sign)
Copy link
Collaborator Author

@babacry babacry Mar 5, 2025

Choose a reason for hiding this comment

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

Previous test case doesn't preserve unitaries of expected $P_{conj}$ and $C^\dagger P C$ for paulistring $P$ and clifford gate $C$ (which is a PauliInteractionGate here).

It's either a deeper issue from the e.g., decompose func of PauliInteractionGate or the implementation error of the old implementation of pass_operations_over.

@babacry babacry changed the title Update pass_operations_over with fixed conjugated_by. Update pass_operations_over with fixed conjugated_by Mar 6, 2025

# Initialize the conjugation of Pc.
conjugated: 'cirq.DensePauliString' = (
dense_pauli_string.DensePauliString(pauli_mask=[identity.I for _ in op.qubits])
* self.coefficient
* ps.coefficient
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bug of the previous pr I submitted. We should roll against ps in the iteration, coeff should be from ps not self, found this by enforcing unitary checks in tests for all conjugated_by tests, previously if expected is given, unitary isn't checked. Fix it here.

pavoljuhas and others added 3 commits March 6, 2025 14:07
* Avoid broken wheels for qcs-sdk-python

The installation of `qcs-sdk-python==0.21.13` fails on Linux Python 3.10 with
`zipfile.BadZipFile: Bad CRC-32 for file 'qcs_sdk/qcs_sdk.cpython-310-x86_64-linux-gnu.so'`

Block new versions of qcs-sdk-python until this is fixed.

Ref: rigetti/qcs-sdk-rust#531

* Also adjust Dockerfile to avoid the bad wheel
@babacry babacry marked this pull request as ready for review March 6, 2025 22:39
@babacry babacry requested review from vtomole and a team as code owners March 6, 2025 22:39
Copy link

codecov bot commented Mar 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.12%. Comparing base (78d30e7) to head (71d70fc).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7123      +/-   ##
==========================================
- Coverage   98.15%   98.12%   -0.04%     
==========================================
  Files        1093     1093              
  Lines       95420    95427       +7     
==========================================
- Hits        93664    93637      -27     
- Misses       1756     1790      +34     

☔ 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.

@NoureldinYosri NoureldinYosri self-assigned this Mar 6, 2025
@babacry
Copy link
Collaborator Author

babacry commented Mar 6, 2025

@Strilanc , I noticed #2351 when I tried to fix conjugation related crashes / failures in PauliString. Fortunately, we have tableau repr of Clifford now, so I rewrite conjugated_by with Clifford gates' tableau.

I figure you have more context of the conjugation, the implementation histories and their usages, wonder if you could help review this and point out the places need to be adjusted? Thanks!

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.

4 participants