Skip to content

Move to method dispatch using act_on for CliffordState, Tableau and ChForm #3456

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

Merged
merged 8 commits into from
Oct 29, 2020

Conversation

smitsanghavi
Copy link
Collaborator

@smitsanghavi smitsanghavi commented Oct 28, 2020

Delete all the now-unneeded methods.

Remaining work: refactor the measurement code

#2948 #2423

@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Oct 28, 2020
@smitsanghavi smitsanghavi marked this pull request as draft October 28, 2020 18:45
@smitsanghavi smitsanghavi marked this pull request as ready for review October 28, 2020 19:32
from cirq.sim import simulator
from cirq.sim.clifford import clifford_tableau, stabilizer_state_ch_form
from cirq.sim.clifford import (act_on_clifford_tableau_args,
Copy link
Contributor

Choose a reason for hiding this comment

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

it seems that we could just import the types from these packages in cirq.sim.clifford.__init__.py and then use them directly. It is the more typical style in Cirq allowing for shorter names and easier code completion. Then you can do from cirq.sim import clifford as clifford and clifford.ActOn...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, and the style guide agrees with you https://cirq.readthedocs.io/en/latest/docs/dev/style.html.

But I feel I have seen the reverse pattern being followed, the most specific subpaths are imported instead of the top-level module (except in tests where it's cirq.Whatever). But I'll at least fix the clifford stuff for this PR to follow the style guide.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Had to shuffle the clifford.__init__.py to avoid circular dependencies. Since these all classes are very closely intertwined, there is a high chance of circular dependencies if using cirq.sim.clifford directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah now that happens. The init module orderings is a dark art.

@smitsanghavi smitsanghavi requested a review from balopat October 29, 2020 01:41
@balopat balopat added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Oct 29, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Oct 29, 2020
@CirqBot CirqBot merged commit 4a3274c into quantumlib:master Oct 29, 2020
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Oct 29, 2020
@smitsanghavi smitsanghavi deleted the useact branch October 29, 2020 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants