-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add the Solovay-Kitaev algorithm to the transpiler passes #5657
Conversation
Transformation pass and code skeleton
Allow any Qiskit gate as basis gates & cleanup
Cleanup the init file
@mtreinish @alexanderivrii all set! Let me know if there's other changes we should add 🙂 |
@Cryoris, thank you for following up on the code reorganization comment. From this point of view, everything looks great, but I haven't really looked at anything else. My understanding was that @mtreinish is doing the actual review. (Of course, I would be happy to help, if required). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is getting close, it'll be really good to get this functionality included. I left a few comments and questions inline, the biggest thing still to do from my PoV are fix the docs so we're actually including them in the build.
adjoint = GateSequence() | ||
adjoint.gates = [gate.inverse() for gate in reversed(self.gates)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to just do GateSequence([gate.inverse() for gate in reversed(self.gates)])
? I think this question holds for a bunch of places we're constructing new GateSequence
objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah because we can more efficiently compute the values that would otherwise be computed from scratch in the __init__
🙂 I'll leave a comment in the code to make that clear
@@ -133,7 +133,7 @@ | |||
DAGLongestPath | |||
|
|||
Synthesis | |||
============= | |||
========= |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add thew transpiler pass to this list and re-export it from this module so people can do from qiskit.transpiler.passes import SolovayKitaev
# if a file, load the dictionary | ||
if isinstance(data, str): | ||
data = np.load(data, allow_pickle=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to keep the save functionality around since the computation is much faster now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's likely still slow for large basis sets/depths or you'd quickly like to restart a calculation 🙂
Removed the SK plugin from the plugin.py docs for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM now thanks for the updates. I pushed a couple commits to fix the docs build and clean things up a bit.
* add sk pass * add utils * add sk skeleton * add example test file * Add implementations initial version * make the test run * fix lint * fix phase, add test * fix the phase calculation * add accuracy test * cleanup of unused methods, append more efficient * Add unittests for Solovay Kitaev Utils * Add unitttests for commutator_decompose * Remove unittests for non-public functions * Fix pylint issues * refactor simplify * fix the test imports * improve and add docstrings * fix lint in testfile * fix lint in utils * fix lint in sk * Add unittests * fix phase, lint and allow basis gates as strings * rm print * fix adjoint and dot * Fix unittests and add docstring to them * move to circuit to GateSequence * Fix assertion * allow caching the basic approxs * Fix bug in append from GateSequence * Remove unnecesssary code * Fix bug in test setup * Add unittests for multiqubit QFT-circuit * Add unittests for QFT with more qubits * make compatible with current tests * Remove unnecessary testcases * Remove unnecessary unittests * Fix bug in unittests * Add release notes * import scipy; scipy.optimize.fsolve() does not work scipy.optimize has to be imported, since it is not imported when scipy is imported * numpy * Update qiskit/transpiler/passes/synthesis/solovay_kitaev_utils.py Co-authored-by: Luciano Bello <[email protected]> * document ValueError * rm unused methods, add GateSequence tests * fix lint * try fixing lint and docs * cleanup * improve readability of math * move commutator_decompose into utils * rm tests that are never run * rm trailing todos and commented code * add default dataset, cleanup tests * rm unused imports * replace diamond norm by trace distance * rm unused imports * fix deprecated use of DAGNode.type * black * really black! * fail nicely * test * attempt to fix default approx path * Move decompositions into py file dict * speed up the basic approx generation * add candidate checking by string further reduces the runtime by a factor of about 1.8! * use a tree for basic approx generation Co-authored-by: Jake Lishman <[email protected]> * fix tests * more speedups! * use kdtree! * refactor: generation as sep. function * remove this masssssive file! * start plugin work * first attempt at synthesis plugin * plugin itself works -- but not when called via transpile or the UnitarySynthesis * unitary synth works! * use class-level method for SolovayKitaev * docstrings * add tests which cover still missing feature * rename to SKSynth and better reno * re-organize files - algorithm to qiskit.synthesis - plugin to qiskit.transpiler.passes.synthesis * cover sklearn-free case * missing future import * move sk pass to transpiler subdirectory * reno & try fixing slow optional module-level imports * attempt 2 to fix sklearn import * comments from Matthew's review * directly construct dag * (previous commit incomplete: missed this here) * add SK to plugin toctree * try fixing sphinx * Apply Jake's comments Removed the SK plugin from the plugin.py docs for now * Fix doc example * Update docs * Fix tests * Fix docs error Co-authored-by: Julien Gacon <[email protected]> Co-authored-by: Luciano Bello <[email protected]> Co-authored-by: Jake Lishman <[email protected]> Co-authored-by: Matthew Treinish <[email protected]>
Summary
Add the Solovay-Kitaev algorithm from https://arxiv.org/abs/quant-ph/0505030 by C. Dawson and M. Nielsen to the transpiler passes.
Details and comments
For example, the following circuit
can be decomposed into
with an L2-error of approximately 0.01.
The code to do the above is:
Preliminary results
Here are some benchmark results for the QFT circuit on three qubits using the basis gate set
h t tdg
and combinations thereof of depth up to 10.Error (in diamond norm):
Runtime:
TODO