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
[runtime] QubitPlacer 2 - RandomDevicePlacer #4719
[runtime] QubitPlacer 2 - RandomDevicePlacer #4719
Changes from all commits
c463b2d
3f81ea3
b82e9cd
02d5c34
e898908
12ae645
db62d94
7f8cd87
cdd9b91
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.
I'd propose to add a method
nodes_to_qubits
in theNamedTopology
class and use it here instead of this hack, especially because the lambda argument in the qubit placer class below is also not serializable.Note that we already have
nodes_to_gridqubits
method onTiltedSquareLattice
and have an open issue to add a similarnodes_to_linequbits
method for theLineTopology
.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.
hmm I don't consider this a hack.
NamedTopology.nodes_to_qubits
may seem obvious for LineTopology and TiltedSquareLattice but (a) maybe that isn't so and (b) it would be non-obvious for other topology types.(a) what if you happened to use a different qubit type, especially for LineTopology. You could have written your circuit using GridQubit(x, 0) or even GridQubit(x, y) for fixed y where you know that that defines an actual list on the device. What if you used NamedQubit's a, b, c, ...,
(b) Imagine additional named topologies that don't have a corresponding qubit type. Consider a hexagonal topology. what would be the nodes_to_qubits function? there are multiple ways to handle this https://www.redblobgames.com/grids/hexagons/ including mapping to gridqubits or defining a new coordinate system.
nodes_to_gridqubits
andnodes_to_linequbits
as non-abstract, non-interface, topology-specific methods makes sense. you can even imagine havingLineTopology.nodes_to_gridqubits
. But I think it would be too tricky to have as part of the NamedTopology interfaceThere 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.
topo_node_to_qubit_func
parameter, i.e. all qubit placement / routing related utilities. It would be nice if we can avoid passing this additional parameter around by putting the mapping in the named topology itself.nodes_to_qubits
method, or by parameterising the topology class with flags to control the placement behavior.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.
The true solution would be to let you define
Circuit
s with arbitrary qubit identifiers, and then you could use integers or tuples of integers or whatever is appropriate for your problem. The current construct is a shim around that. It's like if in classical programming you couldn't use variable names and had to use hardcoded memory addresses. Most of what follows expands on this idea.cirq.Qid
even if that doesn't make complete sense.NaiveQubitPlacer
to run my problem, you'd know what strategy I used. If I gave you a json document withRandomQubitPlacer
and the target topology, you'd know what strategy I used.There's just this annoying thing you need to "integrate out" where I'd like to describe my problem in terms of namedtopology nodes but have to use gridqubit or linequbit
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.
some semi-relevant hearsay #4713