Add Sycamore23 device #2547
Add Sycamore23 device #2547
Conversation
dstrain115
commented
Nov 14, 2019
dstrain115
commented
- 23 qubit subset of the Sycamore 54 qubit layout
- 23 qubit subset of the Sycamore 54 qubit layout
| cg.Sycamore.validate_operation(syc) | ||
| cg.Sycamore.validate_operation(sqrt_iswap) | ||
| device.validate_operation(syc) | ||
| device.validate_operation(sqrt_iswap) |
There was a problem hiding this comment.
I think you need a test for invalid operations to catch the typos mentioned earlier.
There was a problem hiding this comment.
Done. Thanks for the good catch! That will teach me for hurrying out PRs as I am heading out for the evening!
Co-Authored-By: Casey Duckering <cduck@users.noreply.github.com>
Co-Authored-By: Casey Duckering <cduck@users.noreply.github.com>
|
I think that would be a more precise typing for a ParamDictType. sympy
substitutions are weird. It was unexpected that you could resolve (or
substitute) formulas for different formulas in sympy. I think changing the
type would clarify that this is not the intention in cirq. It's probably
just someone (maybe me even?) getting confused between sympy.Symbol and
sympy.Basic.
It seems irrelevant to the current PR though. I'll create a different PR
for it?
…On Thu, Nov 14, 2019 at 10:23 AM Casey Duckering ***@***.***> wrote:
I think your change is correct but I'm not sure about the value of ParamDictType
= Dict[Union[str, sympy.Basic], Union[float, str, sympy.Symbol]]. I would
expect it to be Dict[Union[str, sympy.Symbol], Union[float, str,
sympy.Basic]] (map symbols or symbol names to values, symbol names, or
arbitrary expressions).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2547?email_source=notifications&email_token=AFLRN6HM3WAA2GJOZCSFFADQTWJR3A5CNFSM4JND5GI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEECZVHA#issuecomment-554015388>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AFLRN6H724VQTRXBWEK5T6TQTWJR3ANCNFSM4JND5GIQ>
.
|
|
@dstrain115 I meant to comment on 2550. |
| @@ -17,6 +17,7 @@ | |||
| import cirq | |||
| import cirq.google as cg | |||
| import cirq.google.common_serializers as cgc | |||
| import cirq.google.devices.known_devices as cgdk | |||
There was a problem hiding this comment.
is this how we're supposed to use it?
There was a problem hiding this comment.
Good question. I wasn't sure if we wanted to expose it in the package for cirq.google or not.
Any opinions?
There was a problem hiding this comment.
I personally find the "use the first letter of each submodule" convention very obfuscating and would support any other convention that avoids it :)
The rest of cirq operates under the "put everything in the top-level namespace" convention
There was a problem hiding this comment.
We should expose Sycamore23 from cirq.google, but not from top-level cirq, as it is a google-specific device.
cirq.google.Sycamore23 same as cirq.google.Bristlecone.
You may abbreviate cirq.google to cg, I don't have strong opinions about that other than the confusion of it matching my initials.
|
Ping. Could I get approval on this in the next few days? I would like to commit this before my next import to google3, which I would like to do by the end of the week. Thanks. |
