From 216b02674014af5c3f78e6f944f28029e045edeb Mon Sep 17 00:00:00 2001 From: a-corni Date: Wed, 19 Apr 2023 18:15:06 +0200 Subject: [PATCH 1/8] Defining default _register if MappableRegister --- pulser-core/pulser/sequence/_decorators.py | 15 --- pulser-core/pulser/sequence/_seq_drawer.py | 18 ++-- pulser-core/pulser/sequence/_seq_str.py | 10 ++ pulser-core/pulser/sequence/sequence.py | 87 +++++++++------ tests/test_sequence.py | 117 +++++++++++++-------- 5 files changed, 144 insertions(+), 103 deletions(-) diff --git a/pulser-core/pulser/sequence/_decorators.py b/pulser-core/pulser/sequence/_decorators.py index 509866ade..3ab845ddc 100644 --- a/pulser-core/pulser/sequence/_decorators.py +++ b/pulser-core/pulser/sequence/_decorators.py @@ -97,21 +97,6 @@ def wrapper(self: Sequence, *args: Any, **kwargs: Any) -> Any: return cast(F, wrapper) -def check_allow_qubit_index(func: F) -> F: - """Checks if using qubit indices is allowed.""" - - @wraps(func) - def wrapper(self: Sequence, *args: Any, **kwargs: Any) -> Any: - if not self.is_parametrized() and self.is_register_mappable(): - raise RuntimeError( - f"Sequence.{func.__name__} cannot be called in" - " non-parametrized sequences using a mappable register." - ) - func(self, *args, **kwargs) - - return cast(F, wrapper) - - def mark_non_empty(func: F) -> F: """Marks the sequence as non-empty.""" diff --git a/pulser-core/pulser/sequence/_seq_drawer.py b/pulser-core/pulser/sequence/_seq_drawer.py index 9fbcfeaf5..c01d2c78d 100644 --- a/pulser-core/pulser/sequence/_seq_drawer.py +++ b/pulser-core/pulser/sequence/_seq_drawer.py @@ -323,10 +323,10 @@ def phase_str(phi: float) -> str: # Draw masked register if draw_register: - pos = np.array(seq.register._coords) - if isinstance(seq.register, Register3D): + pos = np.array(seq._register._coords) + if isinstance(seq._register, Register3D): labels = "xyz" - fig_reg, axes_reg = seq.register._initialize_fig_axes_projection( + fig_reg, axes_reg = seq._register._initialize_fig_axes_projection( pos, blockade_radius=35, draw_half_radius=True, @@ -336,10 +336,10 @@ def phase_str(phi: float) -> str: for ax_reg, (ix, iy) in zip( axes_reg, combinations(np.arange(3), 2) ): - seq.register._draw_2D( + seq._register._draw_2D( ax=ax_reg, pos=pos, - ids=seq.register._ids, + ids=seq._register._ids, plane=(ix, iy), masked_qubits=seq._slm_mask_targets, ) @@ -350,16 +350,16 @@ def phase_str(phi: float) -> str: + "-plane" ) - elif isinstance(seq.register, Register): - fig_reg, ax_reg = seq.register._initialize_fig_axes( + elif isinstance(seq._register, Register): + fig_reg, ax_reg = seq._register._initialize_fig_axes( pos, blockade_radius=35, draw_half_radius=True, ) - seq.register._draw_2D( + seq._register._draw_2D( ax=ax_reg, pos=pos, - ids=seq.register._ids, + ids=seq._register._ids, masked_qubits=seq._slm_mask_targets, ) ax_reg.set_title("Masked register", pad=10) diff --git a/pulser-core/pulser/sequence/_seq_str.py b/pulser-core/pulser/sequence/_seq_str.py index f7ee048ba..b94133736 100644 --- a/pulser-core/pulser/sequence/_seq_str.py +++ b/pulser-core/pulser/sequence/_seq_str.py @@ -14,6 +14,7 @@ """Function for representing the sequence in a string.""" from __future__ import annotations +import warnings from typing import TYPE_CHECKING from pulser.pulse import Pulse @@ -30,6 +31,15 @@ def seq_to_str(sequence: Sequence) -> str: delay_line = "t: {}->{} | Delay \n" det_delay_line = "t: {}->{} | Detuned Delay | Detuning: {:.3g} rad/µs\n" for ch, seq in sequence._schedule.items(): + if ( + seq.channel_obj.addressing == "Global" + and sequence.is_register_mappable() + ): + warnings.warn( + "Showing the register for a sequence with a mappable register." + f"Target qubits of channel {ch} will be defined in build.", + UserWarning, + ) basis = sequence.declared_channels[ch].basis full += f"Channel: {ch}\n" first_slot = True diff --git a/pulser-core/pulser/sequence/sequence.py b/pulser-core/pulser/sequence/sequence.py index e44db16f5..53997bd0e 100644 --- a/pulser-core/pulser/sequence/sequence.py +++ b/pulser-core/pulser/sequence/sequence.py @@ -50,8 +50,10 @@ from pulser.parametrized import Parametrized, Variable from pulser.parametrized.variable import VariableItem from pulser.pulse import Pulse +from pulser.register import Register from pulser.register.base_register import BaseRegister, QubitId from pulser.register.mappable_reg import MappableRegister +from pulser.register.register_layout import RegisterLayout from pulser.sequence._basis_ref import _QubitRef from pulser.sequence._call import _Call from pulser.sequence._schedule import _ChannelSchedule, _Schedule, _TimeSlot @@ -120,15 +122,26 @@ def __init__( raise TypeError( f"'device' must be of type 'BaseDevice', not {type(device)}." ) - - # Checks if register is compatible with the device - if isinstance(register, MappableRegister): - device.validate_layout(register.layout) - device.validate_layout_filling(register) + self._is_register_mappable: bool = isinstance( + register, MappableRegister + ) + self._register: BaseRegister + if self._is_register_mappable: + # Checks if register is compatible with the device + map_reg = cast(MappableRegister, register) + n_ids = len(map_reg.qubit_ids) + device.validate_layout(map_reg.layout) + device.validate_layout_filling(map_reg) + self.initial_layout = map_reg.layout + self._register = Register( + dict(zip(map_reg.qubit_ids, map_reg.layout.coords[:n_ids])), + layout=map_reg.layout, + trap_ids=range(n_ids), + ) else: - device.validate_register(register) + self._register = cast(BaseRegister, register) + device.validate_register(self.register) - self._register: Union[BaseRegister, MappableRegister] = register self._device = device self._in_xy: bool = False self._mag_field: Optional[tuple[float, float, float]] = None @@ -164,11 +177,12 @@ def _slm_mask_time(self) -> list[int]: def qubit_info(self) -> dict[QubitId, np.ndarray]: """Dictionary with the qubit's IDs and positions.""" if self.is_register_mappable(): - raise RuntimeError( - "Can't access the qubit information when the register is " - "mappable." + warnings.warn( + "Accessing the qubit information whereas the register can " + "still be modified in build.", + UserWarning, ) - return cast(BaseRegister, self._register).qubits + return self._register.qubits @property def device(self) -> DeviceType: @@ -179,11 +193,12 @@ def device(self) -> DeviceType: def register(self) -> BaseRegister: """Register with the qubit's IDs and positions.""" if self.is_register_mappable(): - raise RuntimeError( - "Can't access the sequence's register because the register " - "is mappable." + warnings.warn( + "Accessing the sequence's register whereas the register can" + "still be modified in build.", + UserWarning, ) - return cast(BaseRegister, self._register) + return self._register @property def declared_channels(self) -> dict[str, Channel]: @@ -281,7 +296,7 @@ def is_register_mappable(self) -> bool: Returns: Whether the register is a MappableRegister. """ - return isinstance(self._register, MappableRegister) + return self._is_register_mappable def is_measured(self) -> bool: """States whether the sequence has been measured.""" @@ -393,11 +408,6 @@ def config_slm_mask(self, qubits: Iterable[QubitId]) -> None: f"The '{self._device}' device does not have an SLM mask." ) - if self.is_register_mappable(): - raise RuntimeError( - "The SLM mask can't be combined with a mappable register." - ) - try: targets = set(qubits) except TypeError: @@ -543,7 +553,12 @@ def check_retarget(ch_obj: Channel) -> bool: else: raise TypeError(ch_match_err) # Initialize the new sequence (works for Sequence subclasses too) - new_seq = type(self)(register=self._register, device=new_device) + new_seq = type(self)( + register=self._register + if not self.is_register_mappable() + else MappableRegister(self._register.layout, *self._register._ids), + device=new_device, + ) # Copy the variables to the new sequence new_seq._variables = self.declared_variables @@ -959,7 +974,6 @@ def target( self._target(qubits, channel) @seq_decorators.store - @seq_decorators.check_allow_qubit_index def target_index( self, qubits: Union[int, Iterable[int], Parametrized], @@ -1055,7 +1069,6 @@ def phase_shift( self._phase_shift(phi, *targets, basis=basis) @seq_decorators.store - @seq_decorators.check_allow_qubit_index def phase_shift_index( self, phi: Union[float, Parametrized], @@ -1158,10 +1171,12 @@ def build( """ if self.is_register_mappable(): if qubits is None: - raise ValueError( - "'qubits' must be specified when the sequence is created " - "with a MappableRegister." - ) + warnings.warn( + "'qubits' not specified while sequence was created with a " + "MappableRegister. Joining Trap coordinates with qubit ids" + " using indices of register._ids.", + ), + self._is_register_mappable = False elif qubits is not None: raise ValueError( @@ -1196,7 +1211,10 @@ def build( self._variables[name]._assign(value) if qubits: - reg = cast(MappableRegister, self._register).build_register(qubits) + map_reg = MappableRegister( + self._register.layout, *self._register._ids + ) + reg = map_reg.build_register(qubits) self._set_register(seq, reg) for call in self._to_build_calls: @@ -1369,9 +1387,9 @@ def draw( ) draw_interp_pts = False if draw_register and self.is_register_mappable(): - raise ValueError( - "Can't draw the register for a sequence without a defined " - "register." + warnings.warn( + "Drawing the register of a sequence with a mappable register.", + UserWarning, ) fig_reg, fig = self._plot( draw_phase_area=draw_phase_area, @@ -1505,7 +1523,9 @@ def _check_qubits_give_ids( else: qubits = cast(Tuple[int, ...], qubits) try: - return {self.register.qubit_ids[index] for index in qubits} + return { + self._register.qubit_ids[index] for index in qubits + } except IndexError: raise IndexError("Indices must exist for the register.") ids = set(cast(Tuple[QubitId, ...], qubits)) @@ -1642,6 +1662,7 @@ def _set_register(self, seq: Sequence, reg: BaseRegister) -> None: " have not been assigned a trap." ) seq._register = reg + seq._is_register_mappable = False seq._qids = qids seq._calls[0] = _Call("__init__", (seq._register, seq._device), {}) diff --git a/tests/test_sequence.py b/tests/test_sequence.py index a08aa6ba7..0f80c04f9 100644 --- a/tests/test_sequence.py +++ b/tests/test_sequence.py @@ -16,7 +16,7 @@ import json from typing import Any from unittest.mock import patch - +from copy import copy import numpy as np import pytest @@ -25,6 +25,7 @@ from pulser.channels import Raman, Rydberg from pulser.devices import Chadoq2, IroiseMVP, MockDevice from pulser.devices._device_datacls import Device, VirtualDevice +from pulser.register.base_register import BaseRegister from pulser.register.mappable_reg import MappableRegister from pulser.register.special_layouts import TriangularLatticeLayout from pulser.sampler import sample @@ -507,7 +508,7 @@ def test_switch_device_up( parametrized=parametrized, mappable_reg=mappable_reg, ) - new_seq = seq1.switch_device(devices[0], strict) + new_seq = copy(seq1.switch_device(devices[0], strict)) build_kwargs = {} if parametrized: build_kwargs["delay"] = 120 @@ -987,11 +988,8 @@ def test_config_slm_mask(device): mapp_reg = TriangularLatticeLayout(20, 5).make_mappable_register(10) fail_seq = Sequence(mapp_reg, device) - with pytest.raises( - RuntimeError, - match="The SLM mask can't be combined with a mappable register.", - ): - fail_seq.config_slm_mask({"q0", "q2", "q4"}) + # Works now + fail_seq.config_slm_mask({"q0", "q2", "q4"}) def test_slm_mask(reg, patch_plt_show): @@ -1179,46 +1177,61 @@ def test_hardware_constraints(reg, patch_plt_show): seq.draw(mode="input+output") -def test_mappable_register(patch_plt_show): +def test_mappable_register(patch_plt_show, reg): layout = TriangularLatticeLayout(100, 5) mapp_reg = layout.make_mappable_register(10) seq = Sequence(mapp_reg, Chadoq2) assert seq.is_register_mappable() + assert isinstance(seq._register, BaseRegister) reserved_qids = tuple([f"q{i}" for i in range(10)]) assert seq._qids == set(reserved_qids) - with pytest.raises(RuntimeError, match="Can't access the qubit info"): + assert np.all(seq._register._coords == layout.coords[:10]) + with pytest.warns(UserWarning, match="Accessing the qubit information"): seq.qubit_info - with pytest.raises( - RuntimeError, match="Can't access the sequence's register" - ): + with pytest.warns(UserWarning, match="Accessing the sequence's register"): seq.register - seq.declare_channel("ryd", "rydberg_global") seq.declare_channel("ram", "raman_local", initial_target="q2") - seq.add(Pulse.ConstantPulse(100, 1, 0, 0), "ryd") + seq.declare_channel("ryd_loc", "rydberg_local") + # No Global channel shown, sequence can be printed without warnings + seq.__str__() + # Warning if sequence has Global channels and a mappable register + seq.declare_channel("ryd_glob", "rydberg_global") + warn_message_global = ( + "Showing the register for a sequence with a mappable register." + + "Target qubits of channel ryd_glob will be defined in build." + ) + with pytest.warns(UserWarning, match=warn_message_global): + seq.__str__() + # Index of mappable register can be accessed + seq.phase_shift_index(np.pi / 4, 2, basis="digital") # 2->"q2" + seq.target_index(8, "ryd_loc") # 8->"q8" + seq.add(Pulse.ConstantPulse(100, 1, 0, 0), "ryd_glob") seq.add(Pulse.ConstantPulse(200, 1, 0, 0), "ram") - assert seq._last("ryd").targets == set(reserved_qids) + seq.add(Pulse.ConstantPulse(100, 1, 0, 0), "ryd_loc") + assert seq._last("ryd_glob").targets == set(reserved_qids) assert seq._last("ram").targets == {"q2"} + assert seq._last("ryd_loc").targets == {"q8"} - with pytest.raises(ValueError, match="Can't draw the register"): + with pytest.warns( + UserWarning, + match="Drawing the register of a sequence with a mappable register", + ): seq.draw(draw_register=True) # Can draw if 'draw_register=False' seq.draw() - with pytest.raises(ValueError, match="'qubits' must be specified"): - seq.build() - with pytest.raises( ValueError, match="targeted but have not been assigned" ): seq.build(qubits={"q0": 1, "q1": 10}) with pytest.warns(UserWarning, match="No declared variables named: a"): - seq.build(qubits={"q2": 20, "q0": 10}, a=5) + seq.build(qubits={"q2": 20, "q0": 10, "q8": 0}, a=5) - seq_ = seq.build(qubits={"q2": 20, "q0": 10}) - assert seq_._last("ryd").targets == {"q2", "q0"} + seq_ = seq.build(qubits={"q2": 20, "q0": 10, "q8": 0}) + assert seq_._last("ryd_glob").targets == {"q0", "q2", "q8"} # Check the original sequence is unchanged assert seq.is_register_mappable() init_call = seq._calls[0] @@ -1226,10 +1239,22 @@ def test_mappable_register(patch_plt_show): assert isinstance(init_call.kwargs["register"], MappableRegister) assert not seq_.is_register_mappable() assert seq_.register == Register( - {"q0": layout.traps_dict[10], "q2": layout.traps_dict[20]} + { + "q0": layout.traps_dict[10], + "q2": layout.traps_dict[20], + "q8": layout.traps_dict[0], + } ) with pytest.raises(ValueError, match="already has a concrete register"): - seq_.build(qubits={"q2": 20, "q0": 10}) + seq_.build(qubits={"q2": 20, "q0": 10, "q8": 0}) + + # Also possible to build the default register + with pytest.warns(UserWarning, match="'qubits' not specified"): + seq.build() + # This time the register of the sequence is modified + assert not seq.is_register_mappable() + assert np.all(seq.register.qubit_ids == reserved_qids) + assert np.all(seq.register._coords == layout.coords[:10]) index_function_non_mappable_register_values: Any = [ @@ -1337,28 +1362,28 @@ def test_non_parametrized_non_mappable_register_index_functions( assert seq.current_phase_ref(expected_target, "digital") == phi -@pytest.mark.parametrize( - index_function_params, index_function_mappable_register_values -) -def test_non_parametrized_mappable_register_index_functions_failure( - register, build_params, index, expected_target -): - seq = Sequence(register, Chadoq2) - seq.declare_channel("ch0", "rydberg_local") - seq.declare_channel("ch1", "raman_local") - phi = np.pi / 4 - with pytest.raises( - RuntimeError, - match="Sequence.target_index cannot be called in" - " non-parametrized sequences", - ): - seq.target_index(index, channel="ch0") - with pytest.raises( - RuntimeError, - match="Sequence.phase_shift_index cannot be called in" - " non-parametrized sequences", - ): - seq.phase_shift_index(phi, index) +# @pytest.mark.parametrize( +# index_function_params, index_function_mappable_register_values +# ) +# def test_non_parametrized_mappable_register_index_functions_failure( +# register, build_params, index, expected_target +# ): +# seq = Sequence(register, Chadoq2) +# seq.declare_channel("ch0", "rydberg_local") +# seq.declare_channel("ch1", "raman_local") +# phi = np.pi / 4 +# with pytest.raises( +# RuntimeError, +# match="Sequence.target_index cannot be called in" +# " non-parametrized sequences", +# ): +# seq.target_index(index, channel="ch0") +# with pytest.raises( +# RuntimeError, +# match="Sequence.phase_shift_index cannot be called in" +# " non-parametrized sequences", +# ): +# seq.phase_shift_index(phi, index) def test_multiple_index_targets(reg): From 2555e36ff902d8174723b11c4ba6e328a467d70e Mon Sep 17 00:00:00 2001 From: a-corni Date: Thu, 20 Apr 2023 11:30:10 +0200 Subject: [PATCH 2/8] FIxing syntax, types and testing config_slm --- pulser-core/pulser/sequence/sequence.py | 8 ++- tests/test_sequence.py | 95 ++++++++++--------------- 2 files changed, 45 insertions(+), 58 deletions(-) diff --git a/pulser-core/pulser/sequence/sequence.py b/pulser-core/pulser/sequence/sequence.py index 53997bd0e..502d2deee 100644 --- a/pulser-core/pulser/sequence/sequence.py +++ b/pulser-core/pulser/sequence/sequence.py @@ -556,7 +556,10 @@ def check_retarget(ch_obj: Channel) -> bool: new_seq = type(self)( register=self._register if not self.is_register_mappable() - else MappableRegister(self._register.layout, *self._register._ids), + else MappableRegister( + cast(RegisterLayout, self._register.layout), + *self._register._ids, + ), device=new_device, ) @@ -1212,7 +1215,8 @@ def build( if qubits: map_reg = MappableRegister( - self._register.layout, *self._register._ids + cast(RegisterLayout, self._register.layout), + *self._register._ids, ) reg = map_reg.build_register(qubits) self._set_register(seq, reg) diff --git a/tests/test_sequence.py b/tests/test_sequence.py index 0f80c04f9..78d5554d8 100644 --- a/tests/test_sequence.py +++ b/tests/test_sequence.py @@ -11,12 +11,14 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from __future__ import annotations + import dataclasses import itertools import json from typing import Any from unittest.mock import patch -from copy import copy + import numpy as np import pytest @@ -27,6 +29,7 @@ from pulser.devices._device_datacls import Device, VirtualDevice from pulser.register.base_register import BaseRegister from pulser.register.mappable_reg import MappableRegister +from pulser.register.register_layout import RegisterLayout from pulser.register.special_layouts import TriangularLatticeLayout from pulser.sampler import sample from pulser.sequence.sequence import _TimeSlot @@ -508,7 +511,7 @@ def test_switch_device_up( parametrized=parametrized, mappable_reg=mappable_reg, ) - new_seq = copy(seq1.switch_device(devices[0], strict)) + new_seq = seq1.switch_device(devices[0], strict) build_kwargs = {} if parametrized: build_kwargs["delay"] = 120 @@ -923,73 +926,53 @@ def test_sequence(reg, device, patch_plt_show): assert str(seq) == str(seq_) -def test_config_slm_mask(device): - reg_s = Register({"q0": (0, 0), "q1": (10, 10), "q2": (-10, -10)}) - seq_s = Sequence(reg_s, device) - +@pytest.mark.parametrize("is_register_mappable", [False, True]) +@pytest.mark.parametrize("qubit_ids", [["q0", "q1", "q2"], [0, 1, 2]]) +def test_config_slm_mask(is_register_mappable, qubit_ids, device): + reg: Register | MappableRegister + trap_ids = [(0, 0), (10, 10), (-10, -10)] + if not is_register_mappable: + reg = Register(dict(zip(qubit_ids, trap_ids))) + else: + reg = MappableRegister( + RegisterLayout(trap_ids + [(0, 10), (0, 20), (0, -10)]), *qubit_ids + ) + print(reg.qubit_ids) + print(reg.layout.coords) + is_str_qubit_id = isinstance(qubit_ids[0], str) + seq = Sequence(reg, device) with pytest.raises(ValueError, match="does not have an SLM mask."): - seq_ = Sequence(reg_s, IroiseMVP) - seq_.config_slm_mask(["q0"]) + seq_ = Sequence(reg, IroiseMVP) + seq_.config_slm_mask(["q0" if is_str_qubit_id else 0]) with pytest.raises(TypeError, match="must be castable to set"): - seq_s.config_slm_mask(0) + seq.config_slm_mask(0) with pytest.raises(TypeError, match="must be castable to set"): - seq_s.config_slm_mask((0)) + seq.config_slm_mask((0)) with pytest.raises(ValueError, match="exist in the register"): - seq_s.config_slm_mask("q0") + seq.config_slm_mask("q0") with pytest.raises(ValueError, match="exist in the register"): - seq_s.config_slm_mask(["q3"]) + seq.config_slm_mask(["q3" if is_str_qubit_id else 3]) with pytest.raises(ValueError, match="exist in the register"): - seq_s.config_slm_mask(("q3",)) + seq.config_slm_mask(("q3" if is_str_qubit_id else 3,)) with pytest.raises(ValueError, match="exist in the register"): - seq_s.config_slm_mask({"q3"}) + seq.config_slm_mask({"q3" if is_str_qubit_id else 3}) with pytest.raises(ValueError, match="exist in the register"): - seq_s.config_slm_mask([0]) + seq.config_slm_mask([0 if is_str_qubit_id else "0"]) with pytest.raises(ValueError, match="exist in the register"): - seq_s.config_slm_mask((0,)) + seq.config_slm_mask((0 if is_str_qubit_id else "0",)) with pytest.raises(ValueError, match="exist in the register"): - seq_s.config_slm_mask({0}) + seq.config_slm_mask({0 if is_str_qubit_id else "0"}) - targets_s = ["q0", "q2"] - seq_s.config_slm_mask(targets_s) - assert seq_s._slm_mask_targets == {"q0", "q2"} - - with pytest.raises(ValueError, match="configured only once"): - seq_s.config_slm_mask(targets_s) - - reg_i = Register({0: (0, 0), 1: (10, 10), 2: (-10, -10)}) - seq_i = Sequence(reg_i, device) - - with pytest.raises(TypeError, match="must be castable to set"): - seq_i.config_slm_mask(0) - with pytest.raises(TypeError, match="must be castable to set"): - seq_i.config_slm_mask((0)) - with pytest.raises(ValueError, match="exist in the register"): - seq_i.config_slm_mask("q0") - with pytest.raises(ValueError, match="exist in the register"): - seq_i.config_slm_mask([3]) - with pytest.raises(ValueError, match="exist in the register"): - seq_i.config_slm_mask((3,)) - with pytest.raises(ValueError, match="exist in the register"): - seq_i.config_slm_mask({3}) - with pytest.raises(ValueError, match="exist in the register"): - seq_i.config_slm_mask(["0"]) - with pytest.raises(ValueError, match="exist in the register"): - seq_i.config_slm_mask(("0",)) - with pytest.raises(ValueError, match="exist in the register"): - seq_i.config_slm_mask({"0"}) - - targets_i = [0, 2] - seq_i.config_slm_mask(targets_i) - assert seq_i._slm_mask_targets == {0, 2} + targets = ["q0" if is_str_qubit_id else 0, "q2" if is_str_qubit_id else 2] + seq.config_slm_mask(targets) + if is_str_qubit_id: + assert seq._slm_mask_targets == {"q0", "q2"} + else: + assert seq._slm_mask_targets == {0, 2} with pytest.raises(ValueError, match="configured only once"): - seq_i.config_slm_mask(targets_i) - - mapp_reg = TriangularLatticeLayout(20, 5).make_mappable_register(10) - fail_seq = Sequence(mapp_reg, device) - # Works now - fail_seq.config_slm_mask({"q0", "q2", "q4"}) + seq.config_slm_mask(targets) def test_slm_mask(reg, patch_plt_show): @@ -1177,7 +1160,7 @@ def test_hardware_constraints(reg, patch_plt_show): seq.draw(mode="input+output") -def test_mappable_register(patch_plt_show, reg): +def test_mappable_register(patch_plt_show): layout = TriangularLatticeLayout(100, 5) mapp_reg = layout.make_mappable_register(10) seq = Sequence(mapp_reg, Chadoq2) From 2f442e8869b2ed50d08297c8101374f474d7ddfd Mon Sep 17 00:00:00 2001 From: a-corni Date: Wed, 26 Apr 2023 09:30:01 +0200 Subject: [PATCH 3/8] Reverting changes on Sequence --- pulser-core/pulser/sequence/sequence.py | 91 +++++++++---------------- 1 file changed, 33 insertions(+), 58 deletions(-) diff --git a/pulser-core/pulser/sequence/sequence.py b/pulser-core/pulser/sequence/sequence.py index 502d2deee..e44db16f5 100644 --- a/pulser-core/pulser/sequence/sequence.py +++ b/pulser-core/pulser/sequence/sequence.py @@ -50,10 +50,8 @@ from pulser.parametrized import Parametrized, Variable from pulser.parametrized.variable import VariableItem from pulser.pulse import Pulse -from pulser.register import Register from pulser.register.base_register import BaseRegister, QubitId from pulser.register.mappable_reg import MappableRegister -from pulser.register.register_layout import RegisterLayout from pulser.sequence._basis_ref import _QubitRef from pulser.sequence._call import _Call from pulser.sequence._schedule import _ChannelSchedule, _Schedule, _TimeSlot @@ -122,26 +120,15 @@ def __init__( raise TypeError( f"'device' must be of type 'BaseDevice', not {type(device)}." ) - self._is_register_mappable: bool = isinstance( - register, MappableRegister - ) - self._register: BaseRegister - if self._is_register_mappable: - # Checks if register is compatible with the device - map_reg = cast(MappableRegister, register) - n_ids = len(map_reg.qubit_ids) - device.validate_layout(map_reg.layout) - device.validate_layout_filling(map_reg) - self.initial_layout = map_reg.layout - self._register = Register( - dict(zip(map_reg.qubit_ids, map_reg.layout.coords[:n_ids])), - layout=map_reg.layout, - trap_ids=range(n_ids), - ) + + # Checks if register is compatible with the device + if isinstance(register, MappableRegister): + device.validate_layout(register.layout) + device.validate_layout_filling(register) else: - self._register = cast(BaseRegister, register) - device.validate_register(self.register) + device.validate_register(register) + self._register: Union[BaseRegister, MappableRegister] = register self._device = device self._in_xy: bool = False self._mag_field: Optional[tuple[float, float, float]] = None @@ -177,12 +164,11 @@ def _slm_mask_time(self) -> list[int]: def qubit_info(self) -> dict[QubitId, np.ndarray]: """Dictionary with the qubit's IDs and positions.""" if self.is_register_mappable(): - warnings.warn( - "Accessing the qubit information whereas the register can " - "still be modified in build.", - UserWarning, + raise RuntimeError( + "Can't access the qubit information when the register is " + "mappable." ) - return self._register.qubits + return cast(BaseRegister, self._register).qubits @property def device(self) -> DeviceType: @@ -193,12 +179,11 @@ def device(self) -> DeviceType: def register(self) -> BaseRegister: """Register with the qubit's IDs and positions.""" if self.is_register_mappable(): - warnings.warn( - "Accessing the sequence's register whereas the register can" - "still be modified in build.", - UserWarning, + raise RuntimeError( + "Can't access the sequence's register because the register " + "is mappable." ) - return self._register + return cast(BaseRegister, self._register) @property def declared_channels(self) -> dict[str, Channel]: @@ -296,7 +281,7 @@ def is_register_mappable(self) -> bool: Returns: Whether the register is a MappableRegister. """ - return self._is_register_mappable + return isinstance(self._register, MappableRegister) def is_measured(self) -> bool: """States whether the sequence has been measured.""" @@ -408,6 +393,11 @@ def config_slm_mask(self, qubits: Iterable[QubitId]) -> None: f"The '{self._device}' device does not have an SLM mask." ) + if self.is_register_mappable(): + raise RuntimeError( + "The SLM mask can't be combined with a mappable register." + ) + try: targets = set(qubits) except TypeError: @@ -553,15 +543,7 @@ def check_retarget(ch_obj: Channel) -> bool: else: raise TypeError(ch_match_err) # Initialize the new sequence (works for Sequence subclasses too) - new_seq = type(self)( - register=self._register - if not self.is_register_mappable() - else MappableRegister( - cast(RegisterLayout, self._register.layout), - *self._register._ids, - ), - device=new_device, - ) + new_seq = type(self)(register=self._register, device=new_device) # Copy the variables to the new sequence new_seq._variables = self.declared_variables @@ -977,6 +959,7 @@ def target( self._target(qubits, channel) @seq_decorators.store + @seq_decorators.check_allow_qubit_index def target_index( self, qubits: Union[int, Iterable[int], Parametrized], @@ -1072,6 +1055,7 @@ def phase_shift( self._phase_shift(phi, *targets, basis=basis) @seq_decorators.store + @seq_decorators.check_allow_qubit_index def phase_shift_index( self, phi: Union[float, Parametrized], @@ -1174,12 +1158,10 @@ def build( """ if self.is_register_mappable(): if qubits is None: - warnings.warn( - "'qubits' not specified while sequence was created with a " - "MappableRegister. Joining Trap coordinates with qubit ids" - " using indices of register._ids.", - ), - self._is_register_mappable = False + raise ValueError( + "'qubits' must be specified when the sequence is created " + "with a MappableRegister." + ) elif qubits is not None: raise ValueError( @@ -1214,11 +1196,7 @@ def build( self._variables[name]._assign(value) if qubits: - map_reg = MappableRegister( - cast(RegisterLayout, self._register.layout), - *self._register._ids, - ) - reg = map_reg.build_register(qubits) + reg = cast(MappableRegister, self._register).build_register(qubits) self._set_register(seq, reg) for call in self._to_build_calls: @@ -1391,9 +1369,9 @@ def draw( ) draw_interp_pts = False if draw_register and self.is_register_mappable(): - warnings.warn( - "Drawing the register of a sequence with a mappable register.", - UserWarning, + raise ValueError( + "Can't draw the register for a sequence without a defined " + "register." ) fig_reg, fig = self._plot( draw_phase_area=draw_phase_area, @@ -1527,9 +1505,7 @@ def _check_qubits_give_ids( else: qubits = cast(Tuple[int, ...], qubits) try: - return { - self._register.qubit_ids[index] for index in qubits - } + return {self.register.qubit_ids[index] for index in qubits} except IndexError: raise IndexError("Indices must exist for the register.") ids = set(cast(Tuple[QubitId, ...], qubits)) @@ -1666,7 +1642,6 @@ def _set_register(self, seq: Sequence, reg: BaseRegister) -> None: " have not been assigned a trap." ) seq._register = reg - seq._is_register_mappable = False seq._qids = qids seq._calls[0] = _Call("__init__", (seq._register, seq._device), {}) From 96ee4e2ed3bfc837711d54bbd254fdb22411d61e Mon Sep 17 00:00:00 2001 From: a-corni Date: Wed, 26 Apr 2023 18:04:07 +0200 Subject: [PATCH 4/8] Set an order in MappableRegister for Sequence op --- .../pulser/json/abstract_repr/serializer.py | 9 +-- pulser-core/pulser/register/mappable_reg.py | 32 +++------ pulser-core/pulser/sequence/sequence.py | 13 +++- tests/test_abstract_repr.py | 19 ++++-- tests/test_register_layout.py | 19 +----- tests/test_sequence.py | 68 +++++++++---------- 6 files changed, 70 insertions(+), 90 deletions(-) diff --git a/pulser-core/pulser/json/abstract_repr/serializer.py b/pulser-core/pulser/json/abstract_repr/serializer.py index 9faa4044f..fb339dfe2 100644 --- a/pulser-core/pulser/json/abstract_repr/serializer.py +++ b/pulser-core/pulser/json/abstract_repr/serializer.py @@ -153,14 +153,7 @@ def convert_targets( og_dim = target_array.ndim if og_dim == 0: target_array = target_array[np.newaxis] - try: - indices = seq.register.find_indices(target_array.tolist()) - # RuntimeError raised when calling seq.register for a MappableRegister - except RuntimeError: - raise NotImplementedError( - "Serialization of sequences with local operations and" - " a mappable register is currently not supported." - ) + indices = seq._register.find_indices(target_array.tolist()) return indices[0] if og_dim == 0 else indices def get_kwarg_default(call_name: str, kwarg_name: str) -> Any: diff --git a/pulser-core/pulser/register/mappable_reg.py b/pulser-core/pulser/register/mappable_reg.py index c3eb22697..f3152dc3e 100644 --- a/pulser-core/pulser/register/mappable_reg.py +++ b/pulser-core/pulser/register/mappable_reg.py @@ -80,10 +80,8 @@ def build_register(self, qubits: Mapping[QubitId, int]) -> BaseRegister: qubit_ids=tuple(register_ordered_qubits.keys()), ) - def find_indices( - self, chosen_ids: set[QubitId], id_list: abcSequence[QubitId] - ) -> list[int]: - """Computes indices of qubits according to a register mapping. + def find_indices(self, id_list: abcSequence[QubitId]) -> list[int]: + """Computes indices of qubits. This can especially be useful when building a Pulser Sequence with a parameter denoting qubits. @@ -92,41 +90,31 @@ def find_indices( Let ``reg`` be a mappable register with qubit Ids "a", "b", "c" and "d". - >>> qubit_map = dict(b=1, a=2, d=0) - >>> reg.find_indices( - >>> qubit_map.keys(), - >>> ["a", "b", "d", "a"]) + >>> reg.find_indices(["a", "b", "d", "a"]) - It returns ``[0, 1, 2, 0]``, following the qubits order of the - mappable register, but keeping only the chosen ones. + It returns ``[0, 1, 3, 0]``, following the qubits order of the + mappable register (defined by qubit_ids). Then, it is possible to use these indices when building a sequence, typically to instanciate an array of variables that can be provided as an argument to ``target_index`` and ``phase_shift_index``. - ``qubit_map`` should be provided when building the sequence, - to tell how to instantiate the register from the mappable register. + When building a sequence and declaring N qubits, their ids should + refer to the first N elements of qubit_id. Args: - chosen_ids: IDs of the qubits that are chosen to - map the MappableRegister id_list: IDs of the qubits to denote. Returns: Indices of the qubits to denote, only valid for the given mapping. """ - if not chosen_ids <= set(self._qubit_ids): + if not set(id_list) <= set(self._qubit_ids): raise ValueError( - "Chosen IDs must be selected among pre-declared qubit IDs." + "The IDs list must be selected among pre-declared qubit IDs." ) - if not set(id_list) <= chosen_ids: - raise ValueError( - "The IDs list must be selected among the chosen IDs." - ) - ordered_ids = [id for id in self.qubit_ids if id in chosen_ids] - return [ordered_ids.index(id) for id in id_list] + return [self.qubit_ids.index(id) for id in id_list] def _to_dict(self) -> dict[str, Any]: return obj_to_dict(self, self._layout, *self._qubit_ids) diff --git a/pulser-core/pulser/sequence/sequence.py b/pulser-core/pulser/sequence/sequence.py index d752c27a4..c779f70d7 100644 --- a/pulser-core/pulser/sequence/sequence.py +++ b/pulser-core/pulser/sequence/sequence.py @@ -959,7 +959,6 @@ def target( self._target(qubits, channel) @seq_decorators.store - @seq_decorators.check_allow_qubit_index def target_index( self, qubits: Union[int, Iterable[int], Parametrized], @@ -1055,7 +1054,6 @@ def phase_shift( self._phase_shift(phi, *targets, basis=basis) @seq_decorators.store - @seq_decorators.check_allow_qubit_index def phase_shift_index( self, phi: Union[float, Parametrized], @@ -1162,6 +1160,13 @@ def build( "'qubits' must be specified when the sequence is created " "with a MappableRegister." ) + elif not self.is_parametrized() and qubits.keys() != set( + self._register.qubit_ids[: len(qubits.keys())] + ): + raise ValueError( + f"'qubits' should contain the {len(qubits.keys())} first " + "elements of the 'qubit_ids' of the MappableRegister." + ) elif qubits is not None: raise ValueError( @@ -1505,7 +1510,9 @@ def _check_qubits_give_ids( else: qubits = cast(Tuple[int, ...], qubits) try: - return {self.register.qubit_ids[index] for index in qubits} + return { + self._register.qubit_ids[index] for index in qubits + } except IndexError: raise IndexError("Indices must exist for the register.") ids = set(cast(Tuple[QubitId, ...], qubits)) diff --git a/tests/test_abstract_repr.py b/tests/test_abstract_repr.py index 2342388a2..79b15f013 100644 --- a/tests/test_abstract_repr.py +++ b/tests/test_abstract_repr.py @@ -625,13 +625,21 @@ def test_mappable_register(self, triangular_lattice): ): seq.to_abstract_repr(var=0) + with pytest.raises( + ValueError, + match="The given 'defaults' produce an invalid sequence.", + ): + seq.to_abstract_repr(var=0, qubits={"q1": 0}) + with pytest.raises(TypeError, match="Did not receive values"): - seq.to_abstract_repr(qubits={"q1": 0}) + seq.to_abstract_repr(qubits={"q0": 0}) + + assert not seq.is_parametrized() - abstract = json.loads(seq.to_abstract_repr(var=0, qubits={"q1": 0})) + abstract = json.loads(seq.to_abstract_repr(var=0, qubits={"q0": 0})) assert abstract["register"] == [ - {"qid": "q0"}, - {"qid": "q1", "default_trap": 0}, + {"qid": "q0", "default_trap": 0}, + {"qid": "q1"}, ] assert abstract["variables"]["var"] == dict(type="int", value=[0]) @@ -722,7 +730,6 @@ def test_default_basis( "basis", "ground-rydberg" ) - @pytest.mark.xfail(reason="Can't get index of mappable register qubits.") @pytest.mark.parametrize( "op,args", [ @@ -875,7 +882,7 @@ def test_deserialize_register(self, layout_coords): def test_deserialize_mappable_register(self): layout_coords = (5 * np.arange(8)).reshape((4, 2)) s = _get_serialized_seq( - register=[{"qid": "q0"}, {"qid": "q1", "default_trap": 2}], + register=[{"qid": "q0", "default_trap": 2}, {"qid": "q1"}], layout={ "coordinates": layout_coords.tolist(), "slug": "test_layout", diff --git a/tests/test_register_layout.py b/tests/test_register_layout.py index de219f04f..d671dc1e2 100644 --- a/tests/test_register_layout.py +++ b/tests/test_register_layout.py @@ -226,23 +226,12 @@ def test_mappable_register_creation(): mapp_reg = tri.make_mappable_register(5) assert mapp_reg.qubit_ids == ("q0", "q1", "q2", "q3", "q4") - assert mapp_reg.find_indices( - {"q2", "q4", "q1"}, ["q4", "q2", "q1", "q2"] - ) == [2, 1, 0, 1] + assert mapp_reg.find_indices(["q4", "q2", "q1", "q2"]) == [4, 2, 1, 2] with pytest.raises( ValueError, match="must be selected among pre-declared qubit IDs" ): - mapp_reg.find_indices( - {"q2", "q4", "q1", "q5"}, ["q4", "q2", "q1", "q2"] - ) - - with pytest.raises( - ValueError, match="must be selected among the chosen IDs" - ): - mapp_reg.find_indices( - {"q2", "q4", "q1"}, ["q4", "q2", "q1", "q2", "q3"] - ) + mapp_reg.find_indices(["q4", "q2", "q1", "q5"]) with pytest.raises( ValueError, match="labeled with pre-declared qubit IDs" @@ -255,6 +244,4 @@ def test_mappable_register_creation(): {"q0": tri.traps_dict[10], "q1": tri.traps_dict[49]} ) names = ["q1", "q0", "q0"] - assert mapp_reg.find_indices(qubit_map.keys(), names) == reg.find_indices( - names - ) + assert mapp_reg.find_indices(names) == reg.find_indices(names) diff --git a/tests/test_sequence.py b/tests/test_sequence.py index 78d5554d8..e48447e2b 100644 --- a/tests/test_sequence.py +++ b/tests/test_sequence.py @@ -27,7 +27,6 @@ from pulser.channels import Raman, Rydberg from pulser.devices import Chadoq2, IroiseMVP, MockDevice from pulser.devices._device_datacls import Device, VirtualDevice -from pulser.register.base_register import BaseRegister from pulser.register.mappable_reg import MappableRegister from pulser.register.register_layout import RegisterLayout from pulser.register.special_layouts import TriangularLatticeLayout @@ -926,19 +925,11 @@ def test_sequence(reg, device, patch_plt_show): assert str(seq) == str(seq_) -@pytest.mark.parametrize("is_register_mappable", [False, True]) @pytest.mark.parametrize("qubit_ids", [["q0", "q1", "q2"], [0, 1, 2]]) -def test_config_slm_mask(is_register_mappable, qubit_ids, device): +def test_config_slm_mask(qubit_ids, device): reg: Register | MappableRegister trap_ids = [(0, 0), (10, 10), (-10, -10)] - if not is_register_mappable: - reg = Register(dict(zip(qubit_ids, trap_ids))) - else: - reg = MappableRegister( - RegisterLayout(trap_ids + [(0, 10), (0, 20), (0, -10)]), *qubit_ids - ) - print(reg.qubit_ids) - print(reg.layout.coords) + reg = Register(dict(zip(qubit_ids, trap_ids))) is_str_qubit_id = isinstance(qubit_ids[0], str) seq = Sequence(reg, device) with pytest.raises(ValueError, match="does not have an SLM mask."): @@ -973,6 +964,15 @@ def test_config_slm_mask(is_register_mappable, qubit_ids, device): with pytest.raises(ValueError, match="configured only once"): seq.config_slm_mask(targets) + mapp_reg = MappableRegister( + RegisterLayout(trap_ids + [(0, 10), (0, 20), (0, -10)]), *qubit_ids + ) + fail_seq = Sequence(mapp_reg, device) + with pytest.raises( + RuntimeError, + match="The SLM mask can't be combined with a mappable register.", + ): + fail_seq.config_slm_mask({trap_ids[0], trap_ids[2]}) def test_slm_mask(reg, patch_plt_show): @@ -1165,16 +1165,16 @@ def test_mappable_register(patch_plt_show): mapp_reg = layout.make_mappable_register(10) seq = Sequence(mapp_reg, Chadoq2) assert seq.is_register_mappable() - assert isinstance(seq._register, BaseRegister) reserved_qids = tuple([f"q{i}" for i in range(10)]) assert seq._qids == set(reserved_qids) - assert np.all(seq._register._coords == layout.coords[:10]) - with pytest.warns(UserWarning, match="Accessing the qubit information"): + with pytest.raises(RuntimeError, match="Can't access the qubit info"): seq.qubit_info - with pytest.warns(UserWarning, match="Accessing the sequence's register"): + with pytest.raises( + RuntimeError, match="Can't access the sequence's register" + ): seq.register - seq.declare_channel("ram", "raman_local", initial_target="q2") + seq.declare_channel("ram", "raman_local", initial_target="q0") seq.declare_channel("ryd_loc", "rydberg_local") # No Global channel shown, sequence can be printed without warnings seq.__str__() @@ -1187,34 +1187,36 @@ def test_mappable_register(patch_plt_show): with pytest.warns(UserWarning, match=warn_message_global): seq.__str__() # Index of mappable register can be accessed - seq.phase_shift_index(np.pi / 4, 2, basis="digital") # 2->"q2" - seq.target_index(8, "ryd_loc") # 8->"q8" + seq.phase_shift_index(np.pi / 4, 0, basis="digital") # 0 -> q0 + seq.target_index(2, "ryd_loc") # 2 -> q2 seq.add(Pulse.ConstantPulse(100, 1, 0, 0), "ryd_glob") seq.add(Pulse.ConstantPulse(200, 1, 0, 0), "ram") seq.add(Pulse.ConstantPulse(100, 1, 0, 0), "ryd_loc") assert seq._last("ryd_glob").targets == set(reserved_qids) - assert seq._last("ram").targets == {"q2"} - assert seq._last("ryd_loc").targets == {"q8"} + assert seq._last("ram").targets == {"q0"} + assert seq._last("ryd_loc").targets == {"q2"} - with pytest.warns( - UserWarning, - match="Drawing the register of a sequence with a mappable register", - ): + with pytest.raises(ValueError, match="Can't draw the register"): seq.draw(draw_register=True) # Can draw if 'draw_register=False' seq.draw() + with pytest.raises(ValueError, match="'qubits' must be specified"): + seq.build() with pytest.raises( ValueError, match="targeted but have not been assigned" ): - seq.build(qubits={"q0": 1, "q1": 10}) + seq.build(qubits={"q1": 1, "q0": 10}) with pytest.warns(UserWarning, match="No declared variables named: a"): - seq.build(qubits={"q2": 20, "q0": 10, "q8": 0}, a=5) + seq.build(qubits={"q2": 20, "q0": 10, "q1": 0}, a=5) + + with pytest.raises(ValueError, match="'qubits' should contain the 3"): + seq.build(qubits={"q2": 20, "q0": 10, "q3": 0}, a=5) - seq_ = seq.build(qubits={"q2": 20, "q0": 10, "q8": 0}) - assert seq_._last("ryd_glob").targets == {"q0", "q2", "q8"} + seq_ = seq.build(qubits={"q2": 20, "q0": 10, "q1": 0}) + assert seq_._last("ryd_glob").targets == {"q0", "q1", "q2"} # Check the original sequence is unchanged assert seq.is_register_mappable() init_call = seq._calls[0] @@ -1224,20 +1226,16 @@ def test_mappable_register(patch_plt_show): assert seq_.register == Register( { "q0": layout.traps_dict[10], + "q1": layout.traps_dict[0], "q2": layout.traps_dict[20], - "q8": layout.traps_dict[0], } ) with pytest.raises(ValueError, match="already has a concrete register"): - seq_.build(qubits={"q2": 20, "q0": 10, "q8": 0}) + seq_.build(qubits={"q2": 20, "q0": 10, "q1": 0}) # Also possible to build the default register - with pytest.warns(UserWarning, match="'qubits' not specified"): + with pytest.raises(ValueError, match="'qubits' must be specified"): seq.build() - # This time the register of the sequence is modified - assert not seq.is_register_mappable() - assert np.all(seq.register.qubit_ids == reserved_qids) - assert np.all(seq.register._coords == layout.coords[:10]) index_function_non_mappable_register_values: Any = [ From 30a5becbbcf878906a845dcc848a66d2b73ad277 Mon Sep 17 00:00:00 2001 From: a-corni Date: Wed, 26 Apr 2023 18:26:16 +0200 Subject: [PATCH 5/8] Imposing order in build of MappableRegister --- pulser-core/pulser/register/mappable_reg.py | 5 +++++ pulser-core/pulser/sequence/sequence.py | 7 ------- tests/test_register_layout.py | 4 ++++ tests/test_sequence.py | 8 ++++---- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/pulser-core/pulser/register/mappable_reg.py b/pulser-core/pulser/register/mappable_reg.py index f3152dc3e..f77526457 100644 --- a/pulser-core/pulser/register/mappable_reg.py +++ b/pulser-core/pulser/register/mappable_reg.py @@ -72,6 +72,11 @@ def build_register(self, qubits: Mapping[QubitId, int]) -> BaseRegister: raise ValueError( "All qubits must be labeled with pre-declared qubit IDs." ) + elif set(chosen_ids) != set(self.qubit_ids[: len(chosen_ids)]): + raise ValueError( + f"'qubits' should contain the {len(qubits.keys())} first " + "elements of the 'qubit_ids'." + ) register_ordered_qubits = { id: qubits[id] for id in self._qubit_ids if id in chosen_ids } diff --git a/pulser-core/pulser/sequence/sequence.py b/pulser-core/pulser/sequence/sequence.py index c779f70d7..648f6c07c 100644 --- a/pulser-core/pulser/sequence/sequence.py +++ b/pulser-core/pulser/sequence/sequence.py @@ -1160,13 +1160,6 @@ def build( "'qubits' must be specified when the sequence is created " "with a MappableRegister." ) - elif not self.is_parametrized() and qubits.keys() != set( - self._register.qubit_ids[: len(qubits.keys())] - ): - raise ValueError( - f"'qubits' should contain the {len(qubits.keys())} first " - "elements of the 'qubit_ids' of the MappableRegister." - ) elif qubits is not None: raise ValueError( diff --git a/tests/test_register_layout.py b/tests/test_register_layout.py index d671dc1e2..1dfc4f4ef 100644 --- a/tests/test_register_layout.py +++ b/tests/test_register_layout.py @@ -237,6 +237,10 @@ def test_mappable_register_creation(): ValueError, match="labeled with pre-declared qubit IDs" ): mapp_reg.build_register({"q0": 0, "q5": 2}) + with pytest.raises( + ValueError, match="'qubits' should contain the 2 first elements" + ): + mapp_reg.build_register({"q0": 0, "q2": 2}) qubit_map = {"q0": 10, "q1": 49} reg = mapp_reg.build_register(qubit_map) diff --git a/tests/test_sequence.py b/tests/test_sequence.py index e48447e2b..e77ef0e89 100644 --- a/tests/test_sequence.py +++ b/tests/test_sequence.py @@ -1213,7 +1213,7 @@ def test_mappable_register(patch_plt_show): seq.build(qubits={"q2": 20, "q0": 10, "q1": 0}, a=5) with pytest.raises(ValueError, match="'qubits' should contain the 3"): - seq.build(qubits={"q2": 20, "q0": 10, "q3": 0}, a=5) + seq.build(qubits={"q2": 20, "q0": 10, "q3": 0}) seq_ = seq.build(qubits={"q2": 20, "q0": 10, "q1": 0}) assert seq_._last("ryd_glob").targets == {"q0", "q1", "q2"} @@ -1259,9 +1259,9 @@ def test_mappable_register(patch_plt_show): index_function_mappable_register_values = [ ( TriangularLatticeLayout(100, 5).make_mappable_register(10), - dict(qubits=dict(q0=1, q4=2, q3=0)), - 2, - "q4", + dict(qubits=dict(q0=1, q2=2, q1=0)), + 1, + "q1", ), ] From 13cf73a7dbe82fbbf533a0b1a99a90ae08da6ba6 Mon Sep 17 00:00:00 2001 From: a-corni Date: Fri, 28 Apr 2023 09:59:17 +0200 Subject: [PATCH 6/8] Adding get_register in Sequence --- .../pulser/json/abstract_repr/serializer.py | 2 +- pulser-core/pulser/sequence/sequence.py | 4 +++ tests/test_sequence.py | 27 +++---------------- 3 files changed, 8 insertions(+), 25 deletions(-) diff --git a/pulser-core/pulser/json/abstract_repr/serializer.py b/pulser-core/pulser/json/abstract_repr/serializer.py index fb339dfe2..eaa2022f5 100644 --- a/pulser-core/pulser/json/abstract_repr/serializer.py +++ b/pulser-core/pulser/json/abstract_repr/serializer.py @@ -153,7 +153,7 @@ def convert_targets( og_dim = target_array.ndim if og_dim == 0: target_array = target_array[np.newaxis] - indices = seq._register.find_indices(target_array.tolist()) + indices = seq.get_register().find_indices(target_array.tolist()) return indices[0] if og_dim == 0 else indices def get_kwarg_default(call_name: str, kwarg_name: str) -> Any: diff --git a/pulser-core/pulser/sequence/sequence.py b/pulser-core/pulser/sequence/sequence.py index 648f6c07c..873c77d63 100644 --- a/pulser-core/pulser/sequence/sequence.py +++ b/pulser-core/pulser/sequence/sequence.py @@ -185,6 +185,10 @@ def register(self) -> BaseRegister: ) return cast(BaseRegister, self._register) + def get_register(self) -> BaseRegister | MappableRegister: + """The atom register on which to apply the pulses.""" + return self._register + @property def declared_channels(self) -> dict[str, Channel]: """Channels declared in this Sequence.""" diff --git a/tests/test_sequence.py b/tests/test_sequence.py index e77ef0e89..10e31f63d 100644 --- a/tests/test_sequence.py +++ b/tests/test_sequence.py @@ -27,6 +27,7 @@ from pulser.channels import Raman, Rydberg from pulser.devices import Chadoq2, IroiseMVP, MockDevice from pulser.devices._device_datacls import Device, VirtualDevice +from pulser.register.base_register import BaseRegister from pulser.register.mappable_reg import MappableRegister from pulser.register.register_layout import RegisterLayout from pulser.register.special_layouts import TriangularLatticeLayout @@ -1165,6 +1166,7 @@ def test_mappable_register(patch_plt_show): mapp_reg = layout.make_mappable_register(10) seq = Sequence(mapp_reg, Chadoq2) assert seq.is_register_mappable() + assert isinstance(seq.get_register(), MappableRegister) reserved_qids = tuple([f"q{i}" for i in range(10)]) assert seq._qids == set(reserved_qids) with pytest.raises(RuntimeError, match="Can't access the qubit info"): @@ -1223,6 +1225,7 @@ def test_mappable_register(patch_plt_show): assert init_call.name == "__init__" assert isinstance(init_call.kwargs["register"], MappableRegister) assert not seq_.is_register_mappable() + assert isinstance(seq_.get_register(), BaseRegister) assert seq_.register == Register( { "q0": layout.traps_dict[10], @@ -1343,30 +1346,6 @@ def test_non_parametrized_non_mappable_register_index_functions( assert seq.current_phase_ref(expected_target, "digital") == phi -# @pytest.mark.parametrize( -# index_function_params, index_function_mappable_register_values -# ) -# def test_non_parametrized_mappable_register_index_functions_failure( -# register, build_params, index, expected_target -# ): -# seq = Sequence(register, Chadoq2) -# seq.declare_channel("ch0", "rydberg_local") -# seq.declare_channel("ch1", "raman_local") -# phi = np.pi / 4 -# with pytest.raises( -# RuntimeError, -# match="Sequence.target_index cannot be called in" -# " non-parametrized sequences", -# ): -# seq.target_index(index, channel="ch0") -# with pytest.raises( -# RuntimeError, -# match="Sequence.phase_shift_index cannot be called in" -# " non-parametrized sequences", -# ): -# seq.phase_shift_index(phi, index) - - def test_multiple_index_targets(reg): test_device = Device( name="test_device", From cb988cdc509545d3af0e298e71bb09dd021fa3dd Mon Sep 17 00:00:00 2001 From: a-corni Date: Fri, 28 Apr 2023 10:20:27 +0200 Subject: [PATCH 7/8] Modify tutorial for mappable register definition --- tutorials/advanced_features/Register Layouts.ipynb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tutorials/advanced_features/Register Layouts.ipynb b/tutorials/advanced_features/Register Layouts.ipynb index fa5a18ea9..01e08ba5a 100644 --- a/tutorials/advanced_features/Register Layouts.ipynb +++ b/tutorials/advanced_features/Register Layouts.ipynb @@ -525,7 +525,7 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "To define the register, we can then call `Sequence.build()`, indicated in the `qubits` argument the map between qubit IDs and trap IDs (note that not all the qubit IDs need to be associated to a trap ID). \n", + "To define the register, we can then call `Sequence.build()`, indicating in the `qubits` argument the map between qubit IDs and trap IDs. Note that not all the qubit IDs need to be associated to a trap ID, and that the qubit IDs have to be defined in their order of appearance in `MappableRegister.qubit_ids` (it is not possible to associate a trap ID to qubit ID \"q4\" if no trap ID was assigned to qubit ID \"q3\").\n", "\n", "In this way, we can build multiple sequences, with only the `Register` changing from one to the other:" ] @@ -536,10 +536,10 @@ "metadata": {}, "outputs": [], "source": [ - "seq1 = seq.build(qubits={\"q0\": 16, \"q1\": 19, \"q4\": 34})\n", + "seq1 = seq.build(qubits={\"q0\": 16, \"q1\": 19, \"q2\": 34})\n", "print(\"First register:\", seq1.register.qubits)\n", "\n", - "seq2 = seq.build(qubits={\"q0\": 0, \"q1\": 15, \"q2\": 20})\n", + "seq2 = seq.build(qubits={\"q0\": 0, \"q2\": 15, \"q1\": 20, \"q3\": 50})\n", "print(\"Second register:\", seq2.register.qubits)" ] } From 29c2f40f68923f6fac5beb2e3b07a95fb58984c9 Mon Sep 17 00:00:00 2001 From: a-corni Date: Tue, 2 May 2023 10:32:27 +0200 Subject: [PATCH 8/8] Taking into account review comments --- .../pulser/json/abstract_repr/serializer.py | 4 +++- pulser-core/pulser/register/mappable_reg.py | 5 +++-- pulser-core/pulser/sequence/sequence.py | 16 ++++++++++++++-- tests/test_register_layout.py | 4 +--- tests/test_sequence.py | 7 ++++++- 5 files changed, 27 insertions(+), 9 deletions(-) diff --git a/pulser-core/pulser/json/abstract_repr/serializer.py b/pulser-core/pulser/json/abstract_repr/serializer.py index eaa2022f5..04084d332 100644 --- a/pulser-core/pulser/json/abstract_repr/serializer.py +++ b/pulser-core/pulser/json/abstract_repr/serializer.py @@ -153,7 +153,9 @@ def convert_targets( og_dim = target_array.ndim if og_dim == 0: target_array = target_array[np.newaxis] - indices = seq.get_register().find_indices(target_array.tolist()) + indices = seq.get_register(include_mappable=True).find_indices( + target_array.tolist() + ) return indices[0] if og_dim == 0 else indices def get_kwarg_default(call_name: str, kwarg_name: str) -> Any: diff --git a/pulser-core/pulser/register/mappable_reg.py b/pulser-core/pulser/register/mappable_reg.py index f77526457..29c789c2c 100644 --- a/pulser-core/pulser/register/mappable_reg.py +++ b/pulser-core/pulser/register/mappable_reg.py @@ -74,8 +74,9 @@ def build_register(self, qubits: Mapping[QubitId, int]) -> BaseRegister: ) elif set(chosen_ids) != set(self.qubit_ids[: len(chosen_ids)]): raise ValueError( - f"'qubits' should contain the {len(qubits.keys())} first " - "elements of the 'qubit_ids'." + f"To declare {len(qubits.keys())} qubits, 'qubits' should " + f"contain the first {len(qubits.keys())} elements of the " + "'qubit_ids'." ) register_ordered_qubits = { id: qubits[id] for id in self._qubit_ids if id in chosen_ids diff --git a/pulser-core/pulser/sequence/sequence.py b/pulser-core/pulser/sequence/sequence.py index 873c77d63..62df7f30d 100644 --- a/pulser-core/pulser/sequence/sequence.py +++ b/pulser-core/pulser/sequence/sequence.py @@ -185,9 +185,21 @@ def register(self) -> BaseRegister: ) return cast(BaseRegister, self._register) - def get_register(self) -> BaseRegister | MappableRegister: + @overload + def get_register(self, include_mappable: Literal[False]) -> BaseRegister: + pass + + @overload + def get_register( + self, include_mappable: Literal[True] + ) -> BaseRegister | MappableRegister: + pass + + def get_register( + self, include_mappable: bool = True + ) -> BaseRegister | MappableRegister: """The atom register on which to apply the pulses.""" - return self._register + return self._register if include_mappable else self.register @property def declared_channels(self) -> dict[str, Channel]: diff --git a/tests/test_register_layout.py b/tests/test_register_layout.py index 1dfc4f4ef..38b76f210 100644 --- a/tests/test_register_layout.py +++ b/tests/test_register_layout.py @@ -237,9 +237,7 @@ def test_mappable_register_creation(): ValueError, match="labeled with pre-declared qubit IDs" ): mapp_reg.build_register({"q0": 0, "q5": 2}) - with pytest.raises( - ValueError, match="'qubits' should contain the 2 first elements" - ): + with pytest.raises(ValueError, match="To declare 2 qubits"): mapp_reg.build_register({"q0": 0, "q2": 2}) qubit_map = {"q0": 10, "q1": 49} diff --git a/tests/test_sequence.py b/tests/test_sequence.py index 10e31f63d..d3eb19540 100644 --- a/tests/test_sequence.py +++ b/tests/test_sequence.py @@ -1167,6 +1167,10 @@ def test_mappable_register(patch_plt_show): seq = Sequence(mapp_reg, Chadoq2) assert seq.is_register_mappable() assert isinstance(seq.get_register(), MappableRegister) + with pytest.raises( + RuntimeError, match="Can't access the sequence's register" + ): + seq.get_register(include_mappable=False) reserved_qids = tuple([f"q{i}" for i in range(10)]) assert seq._qids == set(reserved_qids) with pytest.raises(RuntimeError, match="Can't access the qubit info"): @@ -1214,7 +1218,7 @@ def test_mappable_register(patch_plt_show): with pytest.warns(UserWarning, match="No declared variables named: a"): seq.build(qubits={"q2": 20, "q0": 10, "q1": 0}, a=5) - with pytest.raises(ValueError, match="'qubits' should contain the 3"): + with pytest.raises(ValueError, match="To declare 3 qubits"): seq.build(qubits={"q2": 20, "q0": 10, "q3": 0}) seq_ = seq.build(qubits={"q2": 20, "q0": 10, "q1": 0}) @@ -1226,6 +1230,7 @@ def test_mappable_register(patch_plt_show): assert isinstance(init_call.kwargs["register"], MappableRegister) assert not seq_.is_register_mappable() assert isinstance(seq_.get_register(), BaseRegister) + assert isinstance(seq_.get_register(include_mappable=False), BaseRegister) assert seq_.register == Register( { "q0": layout.traps_dict[10],