Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 metadata property for cirq_google devices. #4869
Add metadata property for cirq_google devices. #4869
Changes from all commits
ad8ec70
a23af63
ce5ab64
9795466
1d0f548
c7edb3d
42e44bf
829286c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Just wondering: this is just a defensive check right? It should never be false?
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.
Not sure, I just copied this from lower down in the module
qid_pairs
function.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.
Why is this (cirq.Gate, type(cirq.Gate))? This seems redundant but I assume there's a good reason for it?
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.
Full disclosure: Things in this module are in pretty rough shape. In tests, the type contracts are somehow getting violated in this constructor and other "non Type[_Gate]" things are making it into this function. I'm working on deprecating
qid_pairs
incirq-core
and there are a few hangups incirq_google
that this PR will help unblock.High level is I want to make sure we keep
cirq-core
pristine andcirq-google
devices limping along in whatever weird state it is in so that we can finish the items here #4744 and then make a more focused effort on cleaning this up knowing thatcirq-core
now gives a solid foundation.This weirdness keeps
cirq-google
behaving, albeit weirdly.RE: backwards compatibility and changing in the future we should make a more focused effort on
SerializableDevice
before we cut our next release, I just don't think in this PR as things are bound to keep changing a lot here before that next release.