Add edges to device

[daxfohl]

Closes #3696

[HGSilveri]

As far as PasqalDevice is concerned, this LGTM. Thanks for the enhancement!

[daxfohl]

Updated the function name to edge_set() because there was a conflicting edges property in graph_device. There is already a qubit_set() function, so following that precedent with the new name.

[mpharrigan]

Not all devices have (only) two qubit gates. How would this work for devices that support multi-qubit operations?

[mpharrigan]

+1 using a name other than "edges"

[daxfohl]

What would be better here? The tuple implementation I had first is error prone because tuples are not symmetric. The implementation with the custom QidPair is symmetric but then the question was raised about whether that's a valid assumption for all devices.

Should we go with a third option, which is back to using tuples, but have both orderings be emitted instead of just one? That would double the output size but allow for directional edges if needed.

Per 3+ qubit gates, I don't think this is where that functionality would go. Once you have the two gate edges, you can feed those into some external function to extract n-gate sequences. I'd say that should be external to the device code. Unless there's something specific about devices and what edges are n-qubit capable?

[mpharrigan]

I don't think any of my questions should be seen as blockers. Just curious if you'd thought about any of them or had any thoughts.

Per 3+ qubit gates, I don't think this is where that functionality would go. Once you have the two gate edges, you can feed those into some external function to extract n-gate sequences. I'd say that should be external to the device code. Unless there's something specific about devices and what edges are n-qubit capable?

Yeah fair. Of course you can always dream up something crazy like if three qubits could do a toffli but you couldn't do a two-qubit interaction between any two of them.

QidPair is a nice addition for order-agnostic pairs. I suspect the main problem will be that there are many locations in the code where we technically should be using such a class but instead use a two-tuple.

For something actionable: I do think the name should be qubit_pairs and not anything to do with edges.

[daxfohl]

Sure, Balint said he wanted to take a look first and think about it, since he opened the original issue. So I'll let this hang out here until then. No rush.

…

On Wed, Apr 14, 2021 at 4:35 PM Matthew Harrigan ***@***.***> wrote: I don't think any of my questions should be seen as blockers. Just curious if you'd thought about any of them or had any thoughts. Per 3+ qubit gates, I don't think this is where that functionality would go. Once you have the two gate edges, you can feed those into some external function to extract n-gate sequences. I'd say that should be external to the device code. Unless there's something specific about devices and what edges are n-qubit capable? Yeah fair. Of course you can always dream up something crazy like if three qubits could do a toffli but you couldn't do a two-qubit interaction between any two of them. QidPair is a nice addition for order-agnostic pairs. I suspect the main problem will be that there are many locations in the code where we technically *should* be using such a class but instead use a two-tuple. For something actionable: I do think the name should be qubit_pairs and not anything to do with edges. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3993 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAG464ISJTMAPOAWHJGNWBTTIYRELANCNFSM42MHYQ2Q> .

[daxfohl]

@balopat ping to look at this whenever you have time, no rush. I'm by no means attached to this PR so if you want to change it completely, or table it until some other things are settled just lmk.

[95-martin-orion]

Paging @balopat for review.

[balopat]

Sorry for making you wait on this for so long. I'm fine with merging this, however some parts might change as the Device strategy gets sorted out.