draft: Fix how wire mapping is handled by the QuantumComputer device

[MarquessV]

I know we aren't 100% set on the expectations around how we set the wire mapping, but I decided to package this up and document what I understand thus far, so when we do pick it back up, we have some context on where it left off. This PR works against a live QPU in my tests, and all the tests pass (besides a few related to validation of the wires parameter, which is expected).

As I see it, there are two issues to clear up here:

1. What we want users to provide via the wires parameter.

The qml.device documentation states this can be either an integer, in which case qubits are addressed by consecutive integers, or an iterable of the preferred labels.

As of this draft, the PR covers those cases by mapping the labels to Qubits available on the device. This should work well in the general case, as quilc will optimize which qubits are selected based on the device ISA.

One use case that Rigetti power users may miss is being able to specify a subset of qubits on the device based on current performance. As far as I know, there is no established pattern for that kind of specification.

2. How does PennyLane interpret the wire mapping, and how should samples be organized so that readout is mapped correctly between qubits and wires.

I tried to remedy this by:

• Using PennyLanes Wires class to build the same labels it would internally, though this may not be necessary.
• The define_wire_map is inherited from PennyLane's QubitDevice class. It isn't used by this plugin internally, it makes sense conceptually that this would be PennyLane's source of truth for the wire mapping, but I haven't double checked it's source code to see.
• No sorting of the qubits in generate_samples. This is working under the assumption that the order of the generated samples must match the order of what define_wire_map returns.

[sc-54125]

[lillian542]

I'm not confident that operator wires are being translated correctly for custom wire labels, see comments. Looks like a simple fix to me, but I'm not confident I know the program format, so take a look and let me know if you agree.

Other than that a couple of small suggestions :)

Let's add this to the change-log as well, since initializing with fewer wires than there are qubits available, and initializing with an integer to define the wires, are both new, - and its also new that wires is a required argument.

[lillian542]

Suggested change

	device_wires = Wires(self.wiring)
	device_wires = Wires(range(self.num_wires))

The purpose of this is to map the Pennylane wires on the operations to the internal wiring. Currently, Wires(self.wiring) just returns the Pennylane wires, since converting a dictionary to a wires object uses the keys of the dictionary, so this just maps custom wire labels to custom wire labels.

I believe we want to preserve the previous behaviour mapping to consecutive integers. I think the if-else clause here may have been a relic - since self.wiring on master is just dict(enumerate(self.qc.qubits())) and self.num_wires is self.num_wires = len(self.qc.qubits()), I can't think of a circumstance where Wires(self.wiring) != Wires(range(self.num_wires)). Correct me if I'm wrong, but seems like we don't want the backend wire labels here, but rather the enumeration of the wires, since its for the operations.

[lillian542]

The current implementation leads to this behaviour, which I believe is incorrect:

With the above suggested change I instead get:

Does that look like the expected program for this circuit? Or have I misunderstood the program format for gates?

[MarquessV]

Yeah, definitely not what we wanted. However, after refactoring around this idea, I ran into another issue.

When updating as suggested, the correct program gets generated:

However, upon processing the samples, pennylane throws an out of bounds exception:

After digging into the implementation of expval, this appears to be because pennylane indexes into the sample array using the label of the device qubit. I would expect pennylane to index into the sample array not by using the label literally, but rather by enumerating the device wire labels and doing a reverse lookup to find it's relative position in the sample array. Is my assumption incorrect here? Is there some other way I should be providing the data? Could this be a bug in pennylane?

[lillian542]

Ah, okay, I misunderstood how you want the program to look. I assumed that the qubit specification for the RX operator would be the index, not the qubit number. In that case, the issue is that we are trying to use this function for two different and incompatible things.

The version I suggested in my initial comment will work for the part of PennyLane where you are encountering an issue, but will not work for generating the program for Rigetti. You've updated it to work for the Rigetti program, but not for PennyLane post processing 😅

There are two separate types of wire mapping we are interested in here. One is the conversion between the wires on the (software) device and consecutive indexing in PennyLane. The other is the conversion between the wires on the (software) device and the qubit numbers expected by the hardware backend. This function used (and is expected by the PennyLane device interface) to do the first thing, and you've updated it to do the second.

In other words, the dictionary generated by this function IS (or at least should be) the reverse lookup table for mapping wire label to relative position in the sample array. It should not include the qubit numbers at all, but only the (software) device wire labels and the consecutive integers. I.e. a 3 wire device with wires=["a", "b", "c"], this function should return {"a": 0, "b": 1, "c":2}.

I don't think the plugin actually needs a custom implementation of this method, it can inherit from the PennyLane device base class. Where it does need a custom implementation is for mapping to hardware instructions, that will need to be done in a separate function, instead of overwriting this existing PennyLane function.

I'm going to make a branch of off this with a proposed solution. It seems like this same assumption was made in the parent class, i.e. that "custom wire labels" (this is what we call it whenever users don't have consecutive integers starting with 0 as their wire labels) in PennyLane don't currently translate to the Rigetti plugin.

Let's see if my suggestion fixes this, and also if it breaks other things, and then take it from there!

[lillian542]

Are you sure you want the operators to be specified by qubit label and not by index? That's not currently the behaviour on master from what I can see.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@ad74b16). Click here to learn what that means.

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #147   +/-   ##
=========================================
  Coverage          ?   98.47%           
=========================================
  Files             ?       10           
  Lines             ?      590           
  Branches          ?        0           
=========================================
  Hits              ?      581           
  Misses            ?        9           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MarquessV]

Yeah, definitely not what we wanted. However, after refactoring around this idea, I ran into another issue.

When updating as suggested, the correct program gets generated:

However, upon processing the samples, pennylane throws an out of bounds exception:

After digging into the implementation of expval, this appears to be because pennylane indexes into the sample array using the label of the device qubit. I would expect pennylane to index into the sample array not by using the label literally, but rather by enumerating the device wire labels and doing a reverse lookup to find it's relative position in the sample array. Is my assumption incorrect here? Is there some other way I should be providing the data? Could this be a bug in pennylane?

[lillian542]

Made this PR against your branch with a suggested fix. Not sure what else (if anything) it will break, I'm having trouble running the tests locally. But have a look and see what you think: MarquessV#2