Open ziofil opened 5 years ago
Hi @ziofil, this is actually intentional behaviour, but specific to the Dgate
. With the displacement gate in particular, it is common to consider the alpha parameter in Cartesian form. However, in other applications (such as optimizing its parameters using TensorFlow), it is easier to consider it in polar form.
Since we can make use of Python's duck typing, we tried to support both forms with the following docstring:
i.e., a
can be a complex number, or alternatively you may provide a
as a real number and also phi
for the complex phase.
Oh! Sorry for not spotting that earlier... bad example 😆 However, also LossChannel accepts a complex parameter without complaining. Is that also intended behaviour?
Early on, we had a discussion internally on how much input validation we wanted to do in Strawberry Fields. We definitely did more input validation in places like the engine (i.e., number of qumodes, etc), but decided to be less stringent for the gates. This was for several reasons:
Too much input validation can become unwieldy, complicates the program, and leads to code duplication, and may unintentionally cut off the user from some valid input edge cases
We decided to take the approach to 'trust the user', that they would read the docstrings, and apply the operations as written, similar to the approach taken by NumPy.
Although, in this case, since the LossChannel
unintentionally seems to work fine with complex parameters, perhaps this is something we should validate against? Unless there is a case (I'm very doubtful) where a complex loss parameter makes sense!
Hello, I noticed there is some type checking on the parameters of, for example, the Decomposition class.
@staticmethod
def _check_p0(p0):
"""Checks that p0 is not symbolic."""
if par_is_symbolic(p0):
raise TypeError(
"The first parameter of a Decomposition is a square matrix, and cannot be symbolic."
)
def __init__(self, par, decomp=True):
self._check_p0(par[0])
super().__init__(par)
self.decomp = decomp
"""bool: If False, try to apply the Decomposition as a single primitive operation
instead of decomposing it."""
I would be interested in adding validation using a similar pattern for the LossChannel. If I do this, however, I would feel it necessary to be consistent in terms of the amount of validation on the gates in general.
Are you open to a changeset including added validation across the board. I would be enthusiastic to write this.
For now, I have implemented the following for this specific case. I will wait to hear from you before I make changes across the board.
def par_is_float(p):
"""Returns True iff p is a floating point parameter instance."""
return isinstance(p, sympy.Float)
@staticmethod
def _type_check(T):
"""Checks that T is a float."""
if par_is_float(T):
raise TypeError(
"The T parameter to a Loss Channel must be a float."
)
def __init__(self, T):
self._type_check(T)
super().__init__([T])
Gates and channels don't check that their arguments are real numbers. Example:
This keeps running without raising an exception.