Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions pulser-core/pulser/channels/base_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,8 @@ def __post_init__(self) -> None:
parameters += local_only
if self.propagation_dir is not None:
raise NotImplementedError(
"'propagation_dir' must be left as None in Local channels."
"'propagation_dir' must be left as None in Local "
f"channels; got {self.propagation_dir}."
)

for param in parameters:
Expand Down Expand Up @@ -444,18 +445,20 @@ def validate_duration(self, duration: int, round_up: bool = True) -> int:
_duration = int(duration)
except (TypeError, ValueError):
raise TypeError(
"duration needs to be castable to an int but "
"type %s was provided" % type(duration)
"'duration' needs to be castable to an int; got "
f"{duration!r} of type {type(duration)}."
)

if duration < self.min_duration:
raise ValueError(
"duration has to be at least " + f"{self.min_duration} ns."
f"'duration' has to be at least {self.min_duration} ns; "
f"got {duration}."
)

if self.max_duration is not None and duration > self.max_duration:
raise ValueError(
"duration can be at most " + f"{self.max_duration} ns."
f"'duration' can be at most {self.max_duration} ns; "
f"got {duration}."
)

if round_up and duration % self.clock_period != 0:
Expand All @@ -482,25 +485,26 @@ def validate_pulse(self, pulse: Pulse) -> None:
amp_samples_np = pulse.amplitude.samples.as_array(detach=True)
if self.max_amp is not None and np.any(amp_samples_np > self.max_amp):
raise ValueError(
"The pulse's amplitude goes over the maximum "
"value allowed for the chosen channel."
"The pulse's amplitude goes over the maximum value allowed "
f"for the chosen channel ({self.max_amp}); got "
f"{amp_samples_np.max()}."
Comment on lines +488 to +490

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should specify the pulse that generated this error.

Suggested change
"The pulse's amplitude goes over the maximum value allowed "
f"for the chosen channel ({self.max_amp}); got "
f"{amp_samples_np.max()}."
"The pulse's amplitude goes over the maximum value allowed "
f"for the chosen channel ({self.max_amp}); got "
f"a maximum amplitude {amp_samples_np.max()} in pulse {pulse!r}."

)
if self.max_abs_detuning is not None and np.any(
np.round(
if self.max_abs_detuning is not None:
abs_detuning = np.round(
np.abs(pulse.detuning.samples.as_array(detach=True)),
decimals=6,
)
> self.max_abs_detuning
):
raise ValueError(
"The pulse's detuning values go out of the range "
"allowed for the chosen channel."
)
if np.any(abs_detuning > self.max_abs_detuning):
raise ValueError(
"The pulse's detuning values go out of the range allowed "
f"for the chosen channel ({self.max_abs_detuning}); got "
f"a maximum absolute value of {abs_detuning.max()}."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, it is important to return the pulse in this error message

Suggested change
f"a maximum absolute value of {abs_detuning.max()}."
f"a maximum absolute detuning of {abs_detuning.max()} in pulse {pulse!r}."

)
avg_amp = np.average(amp_samples_np)
if 0 < avg_amp < self.min_avg_amp:
raise ValueError(
"The pulse's average amplitude is below the chosen "
f"channel's limit ({self.min_avg_amp})."
f"channel's limit ({self.min_avg_amp}); got {avg_amp}."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, it is important to return the pulse here

Suggested change
f"channel's limit ({self.min_avg_amp}); got {avg_amp}."
f"channel's limit ({self.min_avg_amp}); got average amplitude {avg_amp} in pulse {pulse!r}."

)

@property
Expand Down
5 changes: 4 additions & 1 deletion pulser-core/pulser/channels/dmm.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,10 @@ def validate_pulse(
)
# Check that detuning is negative
if np.any(round_detuning > 0):
raise ValueError("The detuning in a DMM must not be positive.")
raise ValueError(
"The detuning in a DMM must not be positive; got a maximum "
f"of {round_detuning.max()}."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe what matters here is the detuning of the pulse (maybe not necessarily the pulse, as it's a DMM)

Suggested change
f"of {round_detuning.max()}."
f"of {round_detuning.max()} in detuning {pulse.detuning!r}."

)
# Check that detuning on each atom is above bottom_detuning
min_round_detuning = np.min(round_detuning)
max_weight = np.max(detuning_map.weights)
Expand Down
6 changes: 3 additions & 3 deletions pulser-core/pulser/channels/eom.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ def __post_init__(self) -> None:
if not isinstance(self.controlled_beams, tuple):
if not isinstance(self.controlled_beams, list):
raise TypeError(
"The 'controlled_beams' must be provided as a tuple "
"or list."
"The 'controlled_beams' must be provided as a tuple or "
f"list, not {type(self.controlled_beams)}."
)
# Convert list to tuple to keep RydbergEOM hashable
object.__setattr__(
Expand All @@ -192,7 +192,7 @@ def __post_init__(self) -> None:
):
raise TypeError(
"Every beam must be one of options of the `RydbergBeam`"
f" enumeration, not {self.limiting_beam}."
f" enumeration, not {beam}."

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Actually, I think it would be more interesting to say which attribute has an incorrect type. Perhaps we could use a dict instead of a chain, having {"limiting_beam":self.limiting_beam, "controlled_beams":self.controlled_beams} and

Suggested change
f" enumeration, not {beam}."
f" enumeration. Got {beam} for attribute {dict_key}."

)

@property
Expand Down
3 changes: 2 additions & 1 deletion pulser-core/pulser/channels/modulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,5 +136,6 @@ def validate_mod_bandwidth(mod_bandwidth: float) -> None:
max_bw := calculate_mod_bandwidth_from_amplitude_rise_time(1)
):
raise NotImplementedError(
f"'mod_bandwidth' must be lower than {max_bw:.0f} MHz"
f"'mod_bandwidth' must be lower than {max_bw:.0f} MHz, not "
f"{mod_bandwidth}."
)
43 changes: 37 additions & 6 deletions tests/test_channels.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ def test_bad_init_local_channel(bad_param, bad_value):
Rydberg.Local(**kwargs)


def test_local_channel_propagation_dir_error():
with pytest.raises(
NotImplementedError,
match=re.escape(
"'propagation_dir' must be left as None in Local channels; "
"got (1, 0, 0)."
),
):
Rydberg.Local(None, None, propagation_dir=(1, 0, 0))


def test_bad_durations():
max_duration, min_duration = 10, 16
with pytest.raises(
Expand Down Expand Up @@ -175,11 +186,23 @@ def test_eigenstates():

def test_validate_duration():
ch = Rydberg.Local(20, 10, min_duration=16, max_duration=1000)
with pytest.raises(TypeError, match="castable to an int"):
with pytest.raises(
TypeError,
match=re.escape(
"'duration' needs to be castable to an int; got 'twenty' of "
"type <class 'str'>."
),
):
ch.validate_duration("twenty")
with pytest.raises(ValueError, match="at least 16 ns"):
with pytest.raises(
ValueError,
match=re.escape("'duration' has to be at least 16 ns; got 10."),
):
ch.validate_duration(10)
with pytest.raises(ValueError, match="at most 1000 ns"):
with pytest.raises(
ValueError,
match=re.escape("'duration' can be at most 1000 ns; got 100000.0."),
):
ch.validate_duration(1e5)
with pytest.warns(UserWarning, match="not a multiple"):
ch.validate_duration(31.4)
Expand Down Expand Up @@ -333,19 +356,27 @@ def test_rise_time_consistency():
(
Pulse.ConstantPulse(100, 1e6, 0, 0),
ValueError,
"amplitude goes over the maximum",
re.escape(
"The pulse's amplitude goes over the maximum value allowed"
f" for the chosen channel ({_eom_rydberg.max_amp}); got"
" 1000000.0."
),
),
(
Pulse.ConstantPulse(100, 0, -1e4, 0),
ValueError,
"detuning values go out of the range",
re.escape(
"The pulse's detuning values go out of the range allowed"
f" for the chosen channel ({_eom_rydberg.max_abs_detuning});"
" got a maximum absolute value of 10000.0."
),
),
(
Pulse.ConstantPulse(100, 0.99e-3, 0, 0),
ValueError,
re.escape(
"average amplitude is below the chosen channel's"
f" limit ({_eom_rydberg.min_avg_amp})"
f" limit ({_eom_rydberg.min_avg_amp}); got 0.00099."
),
),
],
Expand Down
6 changes: 5 additions & 1 deletion tests/test_dmm.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,11 @@ def test_validate_pulse(self, physical_dmm):
# Detuning applied to DMM must be negative
pos_det_pulse = Pulse.ConstantPulse(100, 0, 1e-3, 0)
with pytest.raises(
ValueError, match="The detuning in a DMM must not be positive."
ValueError,
match=re.escape(
"The detuning in a DMM must not be positive; got a maximum"
" of 0.001."
),
):
physical_dmm.validate_pulse(pos_det_pulse)

Expand Down
19 changes: 16 additions & 3 deletions tests/test_eom.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import re

import numpy as np
import pytest

Expand Down Expand Up @@ -57,7 +59,10 @@ def test_bad_value_init_eom(bad_param, bad_value, params):
if bad_param == "mod_bandwidth" and bad_value > 0:
error_type = NotImplementedError
max_bw = calculate_mod_bandwidth_from_amplitude_rise_time(1)
error_message = f"'mod_bandwidth' must be lower than {max_bw:.0f} MHz"
error_message = re.escape(
f"'mod_bandwidth' must be lower than {max_bw:.0f} MHz, not "
f"{bad_value}."
)
else:
error_type = ValueError
error_message = f"'{bad_param}' must be greater than zero"
Expand All @@ -77,9 +82,14 @@ def test_bad_value_init_eom(bad_param, bad_value, params):
)
def test_bad_init_eom_beam(bad_param, bad_value, params):
params[bad_param] = bad_value
# The offending beam is reported, not the limiting beam
bad_beam = bad_value if bad_param == "limiting_beam" else bad_value[0]
with pytest.raises(
TypeError,
match="Every beam must be one of options of the `RydbergBeam`",
match=re.escape(
"Every beam must be one of options of the `RydbergBeam`"
f" enumeration, not {bad_beam}."
),
):
RydbergEOM(**params)

Expand All @@ -88,7 +98,10 @@ def test_bad_controlled_beam(params):
params["controlled_beams"] = set(RydbergBeam)
with pytest.raises(
TypeError,
match="The 'controlled_beams' must be provided as a tuple or list.",
match=re.escape(
"The 'controlled_beams' must be provided as a tuple or list,"
" not <class 'set'>."
),
):
RydbergEOM(**params)

Expand Down
6 changes: 5 additions & 1 deletion tests/test_modulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
# limitations under the License.
"""Tests for the modulation bandwidth utilities."""

import re
import warnings

import numpy as np
Expand Down Expand Up @@ -94,7 +95,10 @@ def test_excessive_mod_bandwidth_raises(self):
max_bw = calculate_mod_bandwidth_from_amplitude_rise_time(1)
with pytest.raises(
NotImplementedError,
match=f"'mod_bandwidth' must be lower than {max_bw:.0f} MHz",
match=re.escape(
f"'mod_bandwidth' must be lower than {max_bw:.0f} MHz, "
f"not {max_bw + 1}."
),
):
validate_mod_bandwidth(max_bw + 1)

Expand Down
4 changes: 2 additions & 2 deletions tests/test_paramseq.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ def test_parametrized_before_eom_mode(mod_device):
with pytest.raises(
ValueError,
match="The pulse's amplitude goes over the maximum "
"value allowed for the chosen channel.",
"value allowed for the chosen channel",
):
seq.enable_eom_mode("ch0", 10000, 0.0)

Expand All @@ -318,7 +318,7 @@ def test_parametrized_before_eom_mode(mod_device):
):
seq.add_eom_pulse("ch0", 200, "0.")

with pytest.raises(ValueError, match="duration has to be at least"):
with pytest.raises(ValueError, match="'duration' has to be at least"):
seq.add_eom_pulse("ch0", 0, 0.0)

var = seq.declare_variable("var", dtype=float, size=None)
Expand Down
11 changes: 9 additions & 2 deletions tests/test_sequence.py
Original file line number Diff line number Diff line change
Expand Up @@ -2123,7 +2123,11 @@ def test_estimate_added_delay_dmm():
seq.add(pulse_0, "ising")
assert seq.estimate_added_delay(det_pulse, "dmm_0") == 0
with pytest.raises(
ValueError, match="The detuning in a DMM must not be positive."
ValueError,
match=re.escape(
"The detuning in a DMM must not be positive; got a maximum"
" of 1.0."
),
):
seq.estimate_added_delay(Pulse.ConstantPulse(100, 0, 1, 0), "dmm_0")
with pytest.raises(
Expand Down Expand Up @@ -3264,7 +3268,10 @@ def test_truncate_delay(reg, device):
seq.truncate(197) # Above current duration, nothing changes
assert seq.get_duration() == 196

with pytest.raises(ValueError, match="duration has to be at least 16 ns"):
with pytest.raises(
ValueError,
match=re.escape("'duration' has to be at least 16 ns; got 15."),
):
seq.truncate(15)

# We add another delay and truncate such that it goes below the minimum
Expand Down
Loading