-
Notifications
You must be signed in to change notification settings - Fork 126
Add Task and Simulation utilities for Discrete Time Crystal Experiments #250
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add Task and Simulation utilities for Discrete Time Crystal Experiments #250
Conversation
1d6ca2f
to
492d7f5
Compare
492d7f5
to
9234092
Compare
@augustehirth please ping me (by tagging my github name in a comment; or just making any comment here) when you push changes and are ready for re-review! Otherwise I don't know. This looks good, but there are now merge conflicts that need to be resolved. |
Merge conflicts should be resolved, as long as the checks pass. Thanks for the review! I'll be sure to comment/tag in the future. |
Hey @mpharrigan just a followup tag since I'm unsure if a comment without a tag is sufficient or not. Thank you for the reviews! |
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 for catching the polish nits; I missed too many.
I think adding coupling gates still needs to swap moments, but has been improved based on feedback nonetheless.
I'm not sure what to assert in the test cases, so they currently serve to do branch coverage for the non-helper functions made available in the module's __init__.py
.
) | ||
operation_list.append(coupling_gate.on(previous_qubit, qubit)) | ||
|
||
# Swap the operation lists, to avoid two-qubit gate overlap |
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 was trying to avoid index math for more pythonic style, and I think I found a way to do that and remove the "previous qubit" update lines (by packaging them into the for
statement).
However, if the number of qubits is odd, then we need an odd number of coupling gates, so it doesn't make sense (I think) to iterate by two instead of one because it would add a clunky check to prevent an out of bounds error at the end of the list.
This approach still needs to swap the moments, in order to alternate which one it adds the gate to. Originally this was done with pair assignment (current_moment, other_moment = other_moment, current_moment
). With the "only one variable assignment per line" restriction, three lines are necessary.
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.
Added a bunch of unit-style tests. They're all sort of samey in that they check that the resulting data is of the right shape and the right range of values instead of any other more proper structure.
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.
Approved after 2 last nits.
Thanks for responding to all the comments!
Last 2 nits resolved, thanks! |
PR 2 of 6 on implementing and graphing Figures 2d through 3d of Observation of Time-Crystalline Eigenstate Order on a Quantum Processor (arxiv:2107.13571)
This PR implements the utility functions for constructing and simulating experiments using DTCTask and CompareDTCTask. It depends on PR #249, which adds the Tasks themselves.