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

Compile to ZZ gate instead of MS for Forte backends when transpiling to native IonQ gates #6973

Merged
merged 8 commits into from
Feb 6, 2025

Conversation

radumarg
Copy link
Contributor

Use ZZ gate instead of MS when transpiling circuits to native IonQ gates for forte backends.

@CirqBot CirqBot added the size: S 10< lines changed <50 label Jan 24, 2025
@radumarg radumarg marked this pull request as ready for review January 30, 2025 09:56
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 98.16%. Comparing base (d589bfc) to head (6fbe497).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cirq-ionq/cirq_ionq/ionq_native_target_gateset.py 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6973      +/-   ##
==========================================
- Coverage   98.17%   98.16%   -0.01%     
==========================================
  Files        1085     1085              
  Lines       94672    94678       +6     
==========================================
  Hits        92942    92942              
- Misses       1730     1736       +6     

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

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

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

LGTM, after addressing the CI issues. The patch below should do so
Thank you for the PR @radumarg !

diff --git a/cirq-ionq/cirq_ionq/ionq_native_target_gateset.py b/cirq-ionq/cirq_ionq/ionq_native_target_gateset.py
index 9c77d0ef..46679b5d 100644
--- a/cirq-ionq/cirq_ionq/ionq_native_target_gateset.py
+++ b/cirq-ionq/cirq_ionq/ionq_native_target_gateset.py
@@ -120,6 +120,9 @@ class IonqNativeGatesetBase(cirq.TwoQubitCompilationTargetGateset):
     def _hadamard(self, qubit):
         return [GPI2Gate(phi=0.25).on(qubit), GPIGate(phi=0).on(qubit)]
 
+    def _cnot(self, *qubits):
+        raise NotImplementedError()
+
     def decompose_all_to_all_connect_ccz_gate(
         self, ccz_gate: 'cirq.CCZPowGate', qubits: Tuple['cirq.Qid', ...]
     ) -> 'cirq.OP_TREE':
@@ -199,6 +202,7 @@ class AriaNativeGateset(IonqNativeGatesetBase):
             GPI2Gate(phi=-1 / 4).on(qubits[0]),
         ]
 
+
 class ForteNativeGateset(IonqNativeGatesetBase):
     """Target IonQ native gateset for compiling circuits.
 

@radumarg
Copy link
Contributor Author

radumarg commented Feb 6, 2025

@pavoljuhas CI issues fixed!

GPI2Gate(phi=1 / 2).on(qubits[0]),
GPI2Gate(phi=-1 / 4).on(qubits[0]),
]
return NotImplemented()
Copy link
Collaborator

Choose a reason for hiding this comment

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

NotImplemented is not callable - it is a constant with special meaning for operator methods (e.g., __eq__).
The correct thing here is to raise the NotImplementedError exception (a different thing to the NotImplemented constant).

Please see details here - https://docs.python.org/3.10/library/constants.html?highlight=notimplemented#NotImplemented

Suggested change
return NotImplemented()
raise NotImplementedError()

@radumarg
Copy link
Contributor Author

radumarg commented Feb 6, 2025

@pavoljuhas there were a string of small mistakes, now it should be fine

@pavoljuhas pavoljuhas enabled auto-merge February 6, 2025 20:11
@pavoljuhas pavoljuhas added this pull request to the merge queue Feb 6, 2025
Merged via the queue into quantumlib:main with commit 14d61c8 Feb 6, 2025
37 of 38 checks passed
@radumarg radumarg deleted the new-backend-name branch February 12, 2025 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: S 10< lines changed <50
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants