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.

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

Transpile to native IonQ gates #6572

Merged
merged 58 commits into from
Oct 28, 2024
Merged

Transpile to native IonQ gates #6572

merged 58 commits into from
Oct 28, 2024

Conversation

radumarg
Copy link
Contributor

Adding support for transpiling circuit to native IonQ gates.

@radumarg
Adding native ZZ gate. Starting work on adding native gateset for Ari…
904fb56 
…a and Forte.
@radumarg
Typo.
8e70ecd
@radumarg
Add todos.
68e864c
@radumarg
Fix __repr__ function.
b7d5a99
@radumarg
Add todo: file will be removed.
c69faf5
@radumarg
More work for transpiling Cirq code to IonQ native gates.
e831a5c
@radumarg
Various work items.
efdc627
@radumarg
More work on ionq_native_target_gateset.
68d5073
@radumarg
Remove IonQAPIDevice.
04ee1f3
@radumarg
Unde delet files.
d2aab39
@radumarg
Undo remove IonQAPIDevice.
785c611
@radumarg
Fix zz unitary test.
dc903bd
@radumarg
Fix some tests.
7490e8a
@radumarg
Fixing specification of phases for native gates. Update transpile code.
0de3d04
@radumarg
Remove old code.
f2ab953
@radumarg
Adding tests.
f748eb3
@radumarg
Fixing some tests.
57af6f5
@radumarg
Adding tests, fixing bugs.
cfc3071
@radumarg
Adding more tests.
559291b
@radumarg
Adding more tests.
09360b6
@radumarg
Bugfixes.
8002090
@radumarg
Adding more tests.
797f7ce
@radumarg
Typo.
a9d73ed
@radumarg
Implement CCZ gate. Add tests for CCZ gate.
8dbf159
@radumarg
Remove dead code.
65dfc86
@radumarg
Correct test names.
92b8064
@radumarg
Remove unused include.
039a54d
@radumarg
Use a shorter decomposition of Hadamard gate.
e2773a1
@radumarg
Update unit tests to test ForteNativeGateset besides AreaNativeGateset.
d9323ff
@radumarg
Adding more unit tests.
ff88dbe
@github-actions github-actions bot closed this Aug 12, 2024
@splch
Copy link
Contributor

splch commented Aug 27, 2024

hi can we reopen this and merge? @dstrain115

@pavoljuhas pavoljuhas reopened this Sep 4, 2024
@pavoljuhas pavoljuhas removed the Stale label Sep 4, 2024
@pavoljuhas pavoljuhas self-assigned this Sep 4, 2024
@github-actions github-actions bot added the Stale label Oct 5, 2024
@splch
Copy link
Contributor

splch commented Oct 8, 2024

hi @pavoljuhas can we get this reviewed please? a few customers are asking for native gate transpilation in cirq

@github-actions github-actions bot removed the Stale label Oct 9, 2024
@pavoljuhas
Copy link
Collaborator

hi @pavoljuhas can we get this reviewed please? a few customers are asking for native gate transpilation in cirq

I am very busy ATM, but will get to it by the end of the week. Hope it is OK.

@splch
Copy link
Contributor

splch commented Oct 10, 2024

hi @pavoljuhas can we get this reviewed please? a few customers are asking for native gate transpilation in cirq

I am very busy ATM, but will get to it by the end of the week. Hope it is OK.

definitely! thanks for the quick response :)

pavoljuhas
pavoljuhas requested changes Oct 17, 2024
View reviewed changes
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.

Please fix the CI-reported problems and the inline comments in the code.

cirq-ionq/cirq_ionq/ionq_native_gates.py Outdated Show resolved Hide resolved
cirq-ionq/cirq_ionq/ionq_native_gates_test.py Outdated Show resolved Hide resolved
cirq-ionq/cirq_ionq/ionq_native_target_gateset.py Show resolved Hide resolved
cirq-ionq/cirq_ionq/ionq_native_target_gateset.py Outdated Show resolved Hide resolved
cirq-ionq/cirq_ionq/ionq_native_target_gateset.py Outdated Show resolved Hide resolved
cirq-ionq/cirq_ionq/ionq_native_target_gateset_test.py Outdated Show resolved Hide resolved
cirq-ionq/cirq_ionq/ionq_native_target_gateset_test.py Outdated Show resolved Hide resolved
cirq-ionq/cirq_ionq/ionq_native_target_gateset_test.py Outdated Show resolved Hide resolved
cirq-ionq/cirq_ionq/json_resolver_cache.py Show resolved Hide resolved
cirq-ionq/cirq_ionq/__init__.py Show resolved Hide resolved
@radumarg
Copy link
Contributor Author

@pavoljuhas Thank you for reviewing the code for this ticket (Transpile to native IonQ gates). The review comments are almost addressed, but it still requires a bit more work :) However meanwhile, I do not see CI-reported problems in this page. I did get in my mailbox some CI report, but it was for a different ticket (Batch circuit submit in cirq_ionq). Perhaps you need to approve the latest workflow, or maybe I am not looking in the correct place? Thank you!

@pavoljuhas
Copy link
Collaborator

@radumarg - I just approved the CI workflows for the last commit - the results should show up at https://github.com/quantumlib/Cirq/pull/6572/checks. Also, please see https://github.com/quantumlib/Cirq/blob/main/docs/dev/development.md#setting-up-an-environment for instructions how to run tests locally.

@radumarg
Copy link
Contributor Author

radumarg commented Oct 23, 2024 •
edited
Loading

@pavoljuhas I have completed the rework and checked-in the changes. I run the CI tests locally, however running test coverage locally with ./check/pytest-and-incremental-coverage does not seem to produce correct test coverage results. Please approve the workflow in order to see if more fixes are required. Thank you!

@codecov Codecov
Copy link

codecov bot commented Oct 23, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.84%. Comparing base (81f66b9) to head (ddfbcc3).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff            @@
##             main    #6572    +/-   ##
========================================
  Coverage   97.84%   97.84%            
========================================
  Files        1077     1079     +2     
  Lines       92791    92976   +185     
========================================
+ Hits        90788    90973   +185     
  Misses       2003     2003

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

@radumarg
Copy link
Contributor Author

@pavoljuhas Looks like everything is green. Are we ready to merge?

@pavoljuhas
Copy link
Collaborator

@pavoljuhas Looks like everything is green. Are we ready to merge?

I think so. I am on a travel, I will give it a last look ASAP.

@pavoljuhas
Order arguments in IonqNativeGatesetBase docstring
492999b 
Also import items from `typing` in one statement.
And clean up redundant "m" from example circuit diagram.
@pavoljuhas
Remove tests of non-public json serialization implementation details
181b83d 
These are already exercised when testing JSON serialization of
AriaNativeGateset and ForteNativeGateset.
@pavoljuhas
Use cirq.q(n) convenience function instead of oneQubitLine, etc.
5ee707c 
No change in the effective test code.
@pavoljuhas
Clean up redundant return None from generator function
ddfbcc3 
Fix its type annotation instead.
@pavoljuhas
Copy link
Collaborator

@radumarg - I made a few minor changes which I pushed to this PR starting at 492999b91

Please check and if OK, I will merge.

radumarg
radumarg commented Oct 27, 2024
View reviewed changes
Copy link
Contributor Author

@radumarg radumarg left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@pavoljuhas Looks good!

@pavoljuhas pavoljuhas merged commit f9ed148 into quantumlib:main Oct 28, 2024
37 checks passed
@pavoljuhas
Copy link
Collaborator

Thank you for implementing these!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pavoljuhas pavoljuhas pavoljuhas approved these changes

@dabacon dabacon Awaiting requested review from dabacon dabacon is a code owner

@ColemanCollins ColemanCollins Awaiting requested review from ColemanCollins ColemanCollins is a code owner

@nakardo nakardo Awaiting requested review from nakardo nakardo is a code owner

@gmauricio gmauricio Awaiting requested review from gmauricio gmauricio is a code owner

@vtomole vtomole Awaiting requested review from vtomole vtomole is a code owner

@cduck cduck Awaiting requested review from cduck

Assignees

@pavoljuhas pavoljuhas

Labels
size: XL lines changed >1000
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@radumarg @splch @pavoljuhas @CirqBot