From 41ac7cbc32beb6bb542bd6d5528b839fa63340e8 Mon Sep 17 00:00:00 2001 From: Jaewon Yun Date: Wed, 2 Sep 2026 20:54:25 -0400 Subject: [PATCH] Report the offending input in the channels module error messages Error messages that only stated a rule now also report the value that broke it, so the user can see what was wrong without reproducing the failure in a debugger. Covers 11 messages in pulser/channels: 7 in base_channel.py, 2 in eom.py, 1 in dmm.py and 1 in modulation.py. Also fixes the beam validation in RydbergEOM, which reported self.limiting_beam instead of the beam being checked. An invalid entry in controlled_beams was therefore reported as the limiting beam, which is usually valid. The three validate_pulse limits now share one shape: the rule, the limit in parentheses, then the value. validate_duration no longer builds its message with %-formatting, and the mod_bandwidth upper bound gains its missing full stop. Messages describing a state rather than a rejected input are left unchanged, since there is no offending value to report. Existing assertions that stopped before the reported value are extended to the end of the message, and the beam validation test now checks the reported beam. --- pulser-core/pulser/channels/base_channel.py | 36 +++++++++-------- pulser-core/pulser/channels/dmm.py | 5 ++- pulser-core/pulser/channels/eom.py | 6 +-- pulser-core/pulser/channels/modulation.py | 3 +- tests/test_channels.py | 43 ++++++++++++++++++--- tests/test_dmm.py | 6 ++- tests/test_eom.py | 19 +++++++-- tests/test_modulation.py | 6 ++- tests/test_paramseq.py | 4 +- tests/test_sequence.py | 11 +++++- 10 files changed, 103 insertions(+), 36 deletions(-) diff --git a/pulser-core/pulser/channels/base_channel.py b/pulser-core/pulser/channels/base_channel.py index 3a4a40aa9..cd3149d55 100644 --- a/pulser-core/pulser/channels/base_channel.py +++ b/pulser-core/pulser/channels/base_channel.py @@ -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: @@ -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: @@ -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()}." ) - 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()}." + ) 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}." ) @property diff --git a/pulser-core/pulser/channels/dmm.py b/pulser-core/pulser/channels/dmm.py index 4737b3dae..c22da66c6 100644 --- a/pulser-core/pulser/channels/dmm.py +++ b/pulser-core/pulser/channels/dmm.py @@ -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()}." + ) # Check that detuning on each atom is above bottom_detuning min_round_detuning = np.min(round_detuning) max_weight = np.max(detuning_map.weights) diff --git a/pulser-core/pulser/channels/eom.py b/pulser-core/pulser/channels/eom.py index c944afef7..13bc34908 100644 --- a/pulser-core/pulser/channels/eom.py +++ b/pulser-core/pulser/channels/eom.py @@ -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__( @@ -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}." ) @property diff --git a/pulser-core/pulser/channels/modulation.py b/pulser-core/pulser/channels/modulation.py index c6cd17dfc..824e23535 100644 --- a/pulser-core/pulser/channels/modulation.py +++ b/pulser-core/pulser/channels/modulation.py @@ -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}." ) diff --git a/tests/test_channels.py b/tests/test_channels.py index 4159d3f7d..655bd75d6 100644 --- a/tests/test_channels.py +++ b/tests/test_channels.py @@ -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( @@ -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 ." + ), + ): 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) @@ -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." ), ), ], diff --git a/tests/test_dmm.py b/tests/test_dmm.py index 5f184bad8..f2d66edee 100644 --- a/tests/test_dmm.py +++ b/tests/test_dmm.py @@ -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) diff --git a/tests/test_eom.py b/tests/test_eom.py index df084bd89..c50b9db6c 100644 --- a/tests/test_eom.py +++ b/tests/test_eom.py @@ -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 @@ -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" @@ -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) @@ -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 ." + ), ): RydbergEOM(**params) diff --git a/tests/test_modulation.py b/tests/test_modulation.py index 94db4a932..4e150461d 100644 --- a/tests/test_modulation.py +++ b/tests/test_modulation.py @@ -13,6 +13,7 @@ # limitations under the License. """Tests for the modulation bandwidth utilities.""" +import re import warnings import numpy as np @@ -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) diff --git a/tests/test_paramseq.py b/tests/test_paramseq.py index 7048a20a6..8de5657c2 100644 --- a/tests/test_paramseq.py +++ b/tests/test_paramseq.py @@ -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) @@ -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) diff --git a/tests/test_sequence.py b/tests/test_sequence.py index aa1428503..7dcf41113 100644 --- a/tests/test_sequence.py +++ b/tests/test_sequence.py @@ -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( @@ -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