Buggy implementation of get_qubit_weight_map

[lvignoli]

On pulser 0.16.1, I experienced ill-defined DMM maps.

(Specs: MacBookPro 2021 M1, python 3.10.2, pulser 0.16.1)

I have tracked down the issue to the ways the weight map is built:

Pulser/pulser-core/pulser/register/weight_maps.py

Lines 74 to 88 in 80cf640

	def get_qubit_weight_map(
	self, qubits: Mapping[QubitId, np.ndarray]
	) -> dict[QubitId, float]:
	"""Creates a map between qubit IDs and the weight on their sites."""
	qubit_weight_map = {}
	coords_arr = self.sorted_coords
	weights_arr = self.sorted_weights
	for qid, pos in qubits.items():
	dists = np.round(
	np.linalg.norm(coords_arr - np.array(pos), axis=1),
	decimals=COORD_PRECISION,
	)
	matches = np.argwhere(dists == 0.0)
	qubit_weight_map[qid] = float(np.sum(weights_arr[matches]))
	return qubit_weight_map

While matches is expected to be a single element list, this is not checked.
Because there is an equality condition on floating point numbers, it may be the empty list.
When it is the empty list, the weight assigned to the qubit in the weight map is 0.0.

Relaxing the decimal precision specified with COORD_PRECISION to say, 3, recovers the correct DMM maps.
Changing the order of the qubits in the register definition recovers a properly defined DMM (!!).

Here is a minimal example to reproduce the issue.
Since the issue comes from comparisons of floats, it may not be reproducible on other devices and architectures.

Code

import dataclasses

import numpy as np
import pulser
from pulser.devices._devices import AnalogDevice
from pulser.register.special_layouts import TriangularLatticeLayout

_dmm = pulser.channels.dmm.DMM(
    clock_period=4,
    min_duration=16,
    max_duration=2**26,
    mod_bandwidth=8,
    bottom_detuning=-2 * np.pi * 20,  # detuning between 0 and -20 MHz
    total_bottom_detuning=-2 * np.pi * 2000,  # total detuning
)


device = dataclasses.replace(
    AnalogDevice,
    dmm_objects=(_dmm,),
    max_sequence_duration=10_000,
)


tri_layout = TriangularLatticeLayout(n_traps=100, spacing=5)


def define_register_with_detuning_weights(sites, labels, weights):
    positions = np.array([tri_layout.traps_dict[i] for i in sites])

    reg = pulser.Register.from_coordinates(positions, labels=labels)

    det_map = reg.define_detuning_map(dict(zip(labels, weights)))
    qubit_weight_map = det_map.get_qubit_weight_map(reg.qubits)

    # Checks that the actual DMM weight is (almost) equal to the provided one.
    for qubit_id, expected_weight in zip(labels, weights):
        try:
            np.testing.assert_almost_equal(qubit_weight_map[qubit_id], expected_weight)
        except AssertionError as e:
            msg = f"qubit {qubit_id}: {e}"
            print(msg)


def main():
    print("Test 1")

    sites = np.array([31, 53, 39, 62, 43, 49, 42, 37, 48, 44, 55, 50])
    labels = np.array(["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l"])
    weights = np.array([0.0, 0.0, 0.0, 0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0])

    define_register_with_detuning_weights(sites, labels, weights)

    print()
    print("Test 2")

    # The first 4 items of each lists are put at the end.
    # These are the 0 weights of the DMM map.

    sites = np.array([43, 49, 42, 37, 48, 44, 55, 50, 31, 53, 39, 62])
    labels = np.array(["e", "f", "g", "h", "i", "j", "k", "l", "a", "b", "c", "d"])
    weights = np.array([1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 0.0, 0.0, 0.0, 0.0])

    define_register_with_detuning_weights(sites, labels, weights)


if __name__ == "__main__":
    main()

Output:

Test 1
qubit e: 
Arrays are not almost equal to 7 decimals
 ACTUAL: 0.0
 DESIRED: 1.0
qubit f: 
Arrays are not almost equal to 7 decimals
 ACTUAL: 0.0
 DESIRED: 1.0

Test 2

[HGSilveri]

Hi @lvignoli , thanks for the detailed bug report! Upon further inspection, I believe your code is doing something unwanted.

When you define your register from your layout, your are doing:

    positions = np.array([tri_layout.traps_dict[i] for i in sites])

    reg = pulser.Register.from_coordinates(positions, labels=labels)

There are two issues with this form:

1. The register won't be able to recognize which layout it came from.
2. Register.from_coordinates has center=True by default, so your coordinates are being modified to center your register around the origin.

At least on my setup, I find that setting center=False is enough to make the issues disappear. Regardless, creating a register from a layout should always be done through:

reg = tri_layout.define_register(*sites, qubit_ids=labels)

If your issues persist after this correction, let me know!

[lvignoli]

Thank @HGSilveri for your quick feedback.

It's true that I used the register layout in an unorthodox way, merely as a proxy for a coordinates of a triangular lattice.

Does this mean that defining a DMM map on a register that does not originates from a layout is undefined behavior?

[HGSilveri]

Does this mean that defining a DMM map on a register that does not originates from a layout is undefined behavior?

It's not undefined per se, it's fine for emulators since they never require an underlying layout so you can just define the detuning map directly from the register. However, what you were doing here is defining a layout and a detuning map from it, and then separately a register. This increases your chances of your qubit coordinates not aligning with the traps, giving you a weight map of mostly zero weights (as you had here).

When you have a layout, it should be the unique source truth for your coordinates, so it is best practice to define both the detuning map and the register from the same layout.

Also, it is recommended to always use RegisterLayout.define_register() whenever defining a register from a layout because only in this way is the register aware of the layout it came from (which is relevant information for QPU execution).

[lvignoli]

However, what you were doing here is defining a layout and a detuning map from it, and then separately a register.

I am precisely not doing that, am I?

 reg = pulser.Register.from_coordinates(positions, labels=labels)
 _ = reg.define_detuning_map(dict(zip(labels, weights)))

The register is defined without any knowledge of the layout, but so is the detuning map: we pass a mapping of qubit IDs to floats.

[lvignoli]

My understanding of the API is that one specifies the qubit IDs, not the traps IDs. And that pulser would take care of mapping correctly the qubit-wise user intent into a correct trap-wise detuning pattern, should there be any notion of traps in the sequence.

[lvignoli]

The register centering part confuses me as well, since it happens before I am specifying any detuning map, and the detuning map is specified by qubit IDs. So as a user this should not be a concern to me, shouldn't it?

[HGSilveri]

@lvignoli You're totally right, I was fooled by the fact setting center=False solved the issue and jumped to a conclusion without carefully looking at the code, my bad.

The register centering part confuses me as well, since it happens before I am specifying any detuning map, and the detuning map is specified by qubit IDs. So as a user this should not be a concern to me, shouldn't it?

Indeed, it shouldn't be a concern to you. I think the reason it affected the test code outcomes is because it introduces changes to the coordinates that make them be incorrectly considered different from their rounded counterparts in get_qubit_weight_map().

I have the changes to fix this pretty much ready, I'll open a PR soon and ping you for review. Sorry again for the confusion on my part!

[lvignoli]

Thanks a lot Henrique for the swift reaction! Having a look right now 😀