-
Notifications
You must be signed in to change notification settings - Fork 621
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
Improve approximate set generation in qml.ops.sk_decomposition
#6855
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6855 +/- ##
==========================================
- Coverage 99.59% 99.59% -0.01%
==========================================
Files 478 478
Lines 45343 45333 -10
==========================================
- Hits 45161 45151 -10
Misses 182 182 ☔ View full report in Codecov by Sentry. |
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.
Thanks @obliviateandsurrender!
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.
Mostly just concerns with readability.
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.
Thanks @obliviateandsurrender. Wonder if there is a better solution than this local/global trie thing going on though.
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 the local/global stuff is still a little hard to read, but the added comments should make future maintenance less painful.
Context: The approximate set for the Solovay-Kitaev decomposition is currently prepared by populating a trie-based data structure (TbDS) with a BFS traversal. This is efficient but misses out on a few future gate sequences because some parent sequences are caught in the equality check condition.
Description of the Change:
max_length
for broader TbDS, we now separate checking for equivalence with a local and globalKDTree
. The local one is updated as the TbDS is being constructed for a given depth, whereas the global one is updated after the construction for a given depth.Benefits: This makes the approximate set decently dense than before, unlocking efficient decomposition for some previously missing edge cases.
Possible Drawbacks: This adds a constant overhead incurred from traversing additional parent nodes and post-build pruning. However, this is negligible; as the approximate set once built is cached.
Related GitHub Issues: [sc-82463] #6131