Consistency for `Error` messages

[LaurentAjdnik]

There are sooooooo many ways to write an Error message.

For instance when starting the message:

• "The number of qubits..."
• "'n_qubits'..."
• "The number of qubits 'n_qubits'..."

Then we have to decide whether to include the parameter value that was provided and the allowed values (see #148). And how.

Even the phrasing for allowed values can vary a lot:

• "...must be below..."
• "...must be smaller than..."
• "...must be less than..."
• "...can not be above..."
• "...must not be above..."
• "...can not exceed..."
• "...must not exceed..."

Right now, along the code, we have a mix of different approaches.

Maybe we should enact rules to ensure consistency, especially since we intend to add lots of checks and hints.

As an option, we could even create an ErrorMessageFactory class.

One among very few functions:

• Signature: ErrorMessageFactory.getMessage(subject, parameter, value, constraint, allowed)
• Example: ErrorMessageFactory.getMessage("The number of qubits", "n_qubits", -1, ErrorMessageFactory.EQUAL_OR_ABOVE, 1)
• Output: "The number of qubits ('n_qubits' = -1) must be 1 or above"

Of course, we would have to identify the common types of constraints and allowed values (single or interval) but it seems feasable.

At the same time, this would allow us, in test functions, to check for full Error messages with exact matches rather than partial matches or regular expressions.

Maybe I'm splitting hairs here, but I would LOVE to discuss this issue. :grin:

[HGSilveri]

Hi @LaurentAjdnik ! Sorry for not getting back to you on this sooner. I'm always in favour of having some systematic way of doing things, so I am on board with this idea. I do think it needs a bit more fleshing out though, so I propose we stand by until you're done with your current issue and then we can discuss it further, in case you're still up for it. Sounds good?

[LaurentAjdnik]

Hi @HGSilveri, here are some more thoughts about it.

TL;DR

• 92 errors raised in the develop branch
• 8 different error types, but mostly TypeError and ValueError
• These two can be further refined, depending on why they are raised
• That leads to error messages that could be made more consistent through some kind or "Error Factory"
• Anyhow, we really need to enact guidelines on how to write error messages because they're currently EXTREMELY diverse!
• Especially since we will certainly add more and more checks everywhere

Diversity

Sometimes this information is given (when applicable), sometimes not, in error messages:

• The name of the parameter
• The meaning of the parameter in plain text
• The value that was provided
• The allowed value(s)

And even when we have the same info, the sentences might be built differently.

Stats

I might have missed a few but I counted, in the develop branch:

Error	Reason	Number
AttributeError		1
IndexError		1
NotImplementedError		7
RuntimeError		1
SystemError		4
TypeError	Instance	12
TypeError	Others	8
ValueError	Interval	1
ValueError	Set	4
ValueError	Threshold	12
ValueError	Others	40
ZeroDivisionError		1
TOTAL		92
---	---	---

AttributeError

• "No attribute named '{name}' in {self}."

IndexError

• "{key} outside of range for '{self.name}'."

NotImplementedError

• "{self.__class__.__name__} does not support modifications to its duration."
• "Can only rotate arrays in 2D."
• "Can only draw register layouts in 2D."
• "Cannot sample system with single-atom state vectors of dimension > 3."
• "Instance or static method serialization is not supported."
• "Needs more than one atom to draw the blockade radius."
• "Sequence with unsupported bases: " + "".join(not_supported)

RuntimeError

• "Sequence.{func.__name__} can't be called in parametrized sequences."

SystemError

• "Can't draw an empty sequence."
• "The sequence has already been measured."
• "The sequence has already been measured. Nothing more can be added."
• "The sequence has been measured, no further changes are allowed."

TypeError / Instance

Raised when a parameter is not an instance of the expected type.

Lots of different phrasings. Should be rewritten or factored.

• "'device' must be of type 'Device'. Import a valid device from 'pulser.devices'."
• "duration needs to be castable to an int but type %s was provided" % type(duration)
• "Given variable 'size' is not of type 'int'."
• "Incompatible type of observable."
• "Invalid data type '{self.dtype}' for Variable."
• "Invalid key type {type(key)} for '{self.name}'."
• "obs_list must be a list of operators"
• "Provided values for variable '{self.name}' must be of type 'str'."
• "pulse input must be of type Pulse, not of type {}.".format(type(pulse))
• "register has to be a pulser.Register instance."
• "Variable's 'name' has to be of type 'str'."
• "The provided sequence has to be a valid "pulser.Sequence instance."

TypeError / Others

Very specific and probably nothing to factor.

• "{!r} is not a valid waveform. Please provide a valid Waveform.".format(waveform))
• "'amplitude' and 'detuning' have to be waveforms."
• "Can't reduce a system in {self._basis_name} to the {reduce_to_basis} basis.")
• "Can't reduce to chosen basis because the population of a state to eliminate is above the allowed tolerance."
• "Did not receive values for variables: " + ", ".join(missing_vars))
• "Failed to automatically adjust one of the pulse's waveforms to the channel duration constraints. Choose a duration that is a multiple of {channel_obj.clock_period} ns."
• "Variable '{self.name}' is not subscriptable."
• "The qubits have to be stored in a dictionary matching qubit ids to position coordinates."

ValueError / Interval

Raised when a parameter does not belong to an interval.

Could be transformed into two ValueError / Threshold checks.

• "sampling_rate must be positive and not larger than 1.0"

ValueError / Set

Raised when a parameter does not belong to a set of allowed values.

Lots of different phrasings. Should be rewritten or factored.

• "basis_name must be 'ground-rydberg', 'digital' or 'all'."
• "'meas_basis' can only be 'ground-rydberg' or 'digital'."
• "meas_basis must be 'ground-rydberg' or 'digital'."
• "'reduce_to_basis' must be 'ground-rydberg' or 'digital', not '{reduce_to_basis}'."

ValueError / Threshold

Raised when a value should be above/below/equal to some allowed threshold (included/exluded).

Lots of different phrasings. Should be rewritten or factored.

• "A waveform has to have a positive duration, not {duration}."
• "All qubit positions must be {}D vectors.".format(self.dimensions)
• "All qubit positions must be {}D vectors.".format(self.dimensions)
• "All qubits must be at most {} μm away from the center of the array.".format(self.max_radial_distance))
• "Can't assign array of size {val.size} to variable of size {self.size}."
• "duration has to be at least {self.min_duration} ns."
• "duration can be at most {self.max_duration} ns."
• "Needs at least two waveforms to form a "CompositeWaveform."
• "Provided time is larger than sequence duration."
• "Provided time is negative."
• "Qubit positions don't respect the minimal distance between atoms for this device.")
• "The pulse's amplitude goes over the maximum value allowed for the chosen channel."
• "The pulse's detuning values go out of the range allowed for the chosen channel."
• "Too many atoms in the array, the device accepts at most {} atoms.".format(self.max_atom_num)
• "sampling_rate is too small, less than 4 data points."

ValueError / Others

So many of them (40) ! I won't write them down here.

ZeroDivisionError

• "Can't divide a waveform by zero."

[HGSilveri]

One other thing: I believe that sometimes the built-in errors are abused, mostly out of laziness to not wanting to create a new error class. I would be in favour of being more strict with their definition and raising a custom Error whenever none of the built-in ones are a good fit. Would you agree?

[LaurentAjdnik]

One other thing: I believe that sometimes the built-in errors are abused, mostly out of laziness to not wanting to create a new error class. I would be in favour of being more strict with their definition and raising a custom Error whenever none of the built-in ones are a good fit. Would you agree?

I agree. Custom errors can be convenient when they make sense functionally.

Here, I feel like MeasurementError is a must-have. It is closely related to quantic concepts. It would replace three occurrences of SystemError.

Maybe also (but not so sure) some kind of DeviceError when we manipulate it in an improper way?

Or perhaps (even less sure) something close to ChannelError and WaveformError for some specific cases?

Otherwise, I wouldn't specialize too much. Most errors are TypeError and ValueError which seem relevant to me.

[LaurentAjdnik]

Back to the original issue of consistency for error messages.

We could rewrite some of them already.

Here are a few rules for ValueError when applied to a threshold:

• Start the sentence with a functional explanation of the parameter
• Include the technical name of the parameter
• Display the given value that caused the error
• Always use must instead of can not, should...
• Use the unambiguous equal to, greater than, greater than or equal to, less than, less than or equal to rather than above, below, at least...
• Negations:
  • (Option 1) Avoid negation: must be greater than or equal to rather than must not be less than
  • (Option 2) Use negation to keep sentences short: must not be less than rather than must be greater than or equal to
  • I personnally favor Option 1
• Display and explain the threshold value
• In any case, make it clear whether the threshold value is allowed or forbidden
• End all messages with a dot
• Use f-strings instead of any other formatting method (%, format()...)

Example:

• Spacing between atoms ('spacing' = 3) must be greater than or equal to the minimal distance allowed by the device (4 µm). OR
• Spacing between atoms ('spacing' = 3) must not be less than the minimal distance allowed by the device (4 µm).

Another example:

• The number of atoms ('n_qubits' = 125) must be less than or equal the max number of atoms allowed by the device (100). OR
• The number of atoms ('n_qubits' = 125) must not be greater than the max number of atoms allowed by the device (100).

What do you think?

[HGSilveri]

I think this is great, I have no objections.

My only concern is the size of the error factory call becoming larger than the written message itself, and perhaps it being hard to decipher for someone reading the source code.

Nonetheless, I'm all for this change, I look forward to seeing what you come up with!

[LaurentAjdnik]

My only concern is the size of the error factory call becoming larger than the written message itself, and perhaps it being hard to decipher for someone reading the source code.

I totally agree. Sorry for not being more explicit. My "we could rewrite some of them already" meant "without a factory".

Nonetheless, I'm all for this change, I look forward to seeing what you come up with!

I'll start working on it soon, first with TypeError and ValueError (intervals / thresholds).