mabuchilab / QNET

Computer algebra package for quantum mechanics and photonic quantum networks
https://qnet.readthedocs.io/
MIT License
71 stars 23 forks source link

IdentityOperator eq/hash mismatch #63

Closed goerz closed 6 years ago

goerz commented 6 years ago
>>> IdentityOperator == 1
True
>>> hash(IdentityOperator) == hash(1)
False

Other Singletons might have similar problems. We should carefully consider what the consequences of hash(IdentityOperator) == hash(1) are (caching!) and decide whether we want/need IdentityOperator == 1

goerz commented 6 years ago

We should also ensure that

>>> IdentityOperator is 1
False

and that 1, 1.0 can be used in any expression in place of an IdentityOperator. This should deal with any caching problems.

danielwe commented 6 years ago
>>> IdentityOperator == 1
True
>>> hash(IdentityOperator) == hash(1)
False

As we also discussed in #51, this violates the python data model and should be fixed, but I don't know what the best solution is.

I was all into supporting equality with objects of different types using automatic dispatch to reflected comparison methods when working on #55 and sympy/sympy#13078, but I didn't realize then that if your objects are hashable, you also need to enforce equal hashes -- which essentially means that the __hash__ method needs to have the same structure as the __eq__ method, and do something special in all cases where __eq__ does.

For example, ScalarTimesOperator has the following __eq__:

def __eq__(self, other):
    if (self.term is IdentityOperator and
            isinstance(other, SCALAR_TYPES)):
        return self.coeff == other
    return super().__eq__(other)

which means it needs the following __hash__:

def __hash__(self):
    if self.term is IdentityOperator:
        return hash(self.coeff)
    return super().__hash__()

(At least I think that would do the job, although I'm not convinced I currently realize how deep this rabbit hole really is.)

I don't know if this is the way to go or if we should give up on equalities across different types, at least for hashable objects. After all, as soon as hashability enters the game it is impliicitly understood that == means equality in a sufficiently strict sense that the two objects are interchangeable as dictionary keys and set members, etc. -- conventionally this is taken to require the identical type and identical values for selected attributes (this convention is of course not universal -- for example 2 == 2.0 is true).

In packages where there exists a notion of mathematical equivalence separate from plain structural equality as a class instance, I think a common pattern is to use == strictly for the structural equality (the convention described above), and define a separate equals method for the mathematical comparison. For example, in a primitive CAS you might see

>>>  x * x == x ** 2
False
>>> (x * x).equals(x ** 2)
True

if x * x and x ** 2 return different internal representations.

I think there is a module or two in sympy that tries (tried?) to follow this pattern, but it hasn't caught on universally, and == is quite commonly used in the mathematical sense.

goerz commented 6 years ago

Yeah, exactly, every __eq__ needs a matching hash. I do find it a bit scary that the identity/zero classes from the various algebras would not be distinguishable when used as dictionary keys. Not really quite sure what to do, either. But whatever we do, we need to ensure the data model is correct.

It would be worthwile to figure out the the potentially bad consequences for IdentityOperator != 1 etc, to see whether that might be something we could give up. Something like operator_symbol + 1 resulting in operator_symbol + IdentityOperator could certainly still work, via the various create rules, even if 1 != IdentityOperator. Probably, the most realistic application is checking whether an expression is zero in an abstract sense. Maybe every Expression subtype should have a class attribute pointing to the object that is the zero/identity for that particular algebra? Because then the check would be expr == expr.algebraic_zero, or expr == expr.algebraic_identity for comparing against 1.

Right now, I think I might be leaning towards giving up the equality, but it probably requires further contemplation.

goerz commented 6 years ago

One problem is that Bra * Ket thinks it's an operator right now, when it's really a scalar that one would definitely want to compare to 0 or 1. We probably have to think a bit about Ket-Bra, Bra-Ket, and Bra-Op-Ket (which should be a special structure, too) should be handled correctly, as a separate issue. We might need a Scalar type as a subtype of Expression. Bra-Kets would be of type Scalar, and it would also be a wrapper around the actual scalars (numbers or symbols). Something to think about.

goerz commented 6 years ago

Quoting from https://github.com/mabuchilab/QNET/commit/4c0273b38437302526457c90a142efd465d8addd:

With the implementation of the scalar algebra it turns out that the way to go is to have only the scalar Zero and One equal to 0 and 1. In every other algebra, the neutral elements have no relation to the scalar 0 and 1, or to the neutral elements of other algebras.

However, elements of "quantum" algebras (anything that has a Hilbert space, including in fact the scalar algebra on the TrivialSpace) have an is_zero attribute to compare to the abstract zero.