Implement (anti-)Hermitian assumption for symbols (issue 3151). #1158

Merged
merged 16 commits into from Aug 13, 2012

9 participants

@jrioux
SymPy member

Introduce adjoint() for Hermite conjugation and corresponding Hermitian (symbol is self-adjoint) and skew-Hermitian (symbol is the negative of its adjoint) assumptions.

A = Symbol('A', hermitian=True)
B = Symbol('B', antihermitian=True)
adjoint(A)
A
adjoint(B)
-B
adjoint(AB)
-A
B
adjoint(BB)
B
*2

(edit: changed conjugate to adjoint in the above)

http://code.google.com/p/sympy/issues/detail?id=3151

@rlamy
SymPy member

This definition of "hermitian" is problematic because, in sympy, conjugate is complex conjugation, not Hermite conjugation. This means that a complex self-adjunct matrix wouldn't be "hermitian".

@jrioux
SymPy member

Then sympy needs a proper adjoint() function. Only real=True would imply conjugate(a)==a and hermitian=True would imply adjoint(a)==a. Do you agree?

@jrioux
SymPy member

I've restored the meaning of conjugate() to complex conjugation and implemented adjoint() for Hermite conjugation.

@jrioux
SymPy member

Passes tests and doctests on 32- and 64-bit python 2.7.0

@rlamy
SymPy member

Actually, there's already a kind of adjoint() operator in the quantum module, it's called Dagger. We should avoid duplication, but I'm not sure how, exactly.

@ellisonbg : What do you think?

@jrioux
SymPy member

Thanks for the pointer. Either name could be used but the usefulness of the Dagger()/adjoint() class is not limited to just quantum physics or even to physics. I had a look at the quantum module, and it has a distinct HermitianOperator class that subclasses Operator and overwrite _eval_dagger in order to assume self-adjoint. Since Operator subclasses QExpr, which subclasses Expr, it would be easy to implement HermitianOperator by simply setting the new Hermitian assumption.

@ellisonbg
SymPy member

The Dagger class in sympy.physics.quantum.dagger.py should be moved out of there and into a more general location. There are only a few things that are specific to the quantum stuff in the Dagger class and those could be removed. Let's definitely not reinvent this functionality in a new adjoint class. A few points:

  • It should be called Dagger so as not to break the quantum stuff. If you really feel it should be called adjoint, then let's make an alias in sympy.physics.quantum to Dagger.
  • I think it should be an Expr with a first-cap name not a Function with a lower case name.
  • It should be available as sympy.physics.quantum.Dagger still.

I do still think there should be a HermitianOperator class, but it could be adapated to use the new assumptions.

@jrioux
SymPy member

I did this now. It should be transparent to users of physics.quantum. Passes doctests and tests on python 2.7.0

@ellisonbg ellisonbg commented on the diff Mar 27, 2012
sympy/physics/quantum/dagger.py
@@ -75,73 +71,13 @@ class Dagger(Expr):
[1] http://en.wikipedia.org/wiki/Hermitian_transpose
"""
- def __new__(cls, arg, **old_assumptions):
- # Return the dagger of a sympy Matrix immediately.
- if isinstance(arg, (Matrix, numpy_ndarray, scipy_sparse_matrix)):
@ellisonbg
SymPy member

I don't see where this logic is now. I think you left it out.

@jrioux
SymPy member
jrioux added a note Mar 28, 2012

It's below, we just call arg.conjugate().transpose() whenever possible.

@ellisonbg
SymPy member
@jrioux
SymPy member
jrioux added a note Mar 29, 2012

All tests pass including the numpy and scipy arrays.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ellisonbg ellisonbg and 1 other commented on an outdated diff Mar 27, 2012
sympy/physics/quantum/dagger.py
@@ -75,73 +71,13 @@ class Dagger(Expr):
[1] http://en.wikipedia.org/wiki/Hermitian_transpose
"""
- def __new__(cls, arg, **old_assumptions):
- # Return the dagger of a sympy Matrix immediately.
- if isinstance(arg, (Matrix, numpy_ndarray, scipy_sparse_matrix)):
- return matrix_dagger(arg)
- arg = sympify(arg)
- r = cls.eval(arg)
- if isinstance(r, Expr):
- return r
- #make unevaluated dagger commutative or non-commutative depending on arg
@ellisonbg
SymPy member

Does the new adjoint class properly set the commutative keyword arg if its arguments are non-commutative? That is very important.

@jrioux
SymPy member
jrioux added a note Mar 28, 2012

Yes, that's automatic. Since a test was missing I added one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ellisonbg ellisonbg commented on the diff Mar 27, 2012
sympy/physics/quantum/dagger.py
- or arg.is_Function or arg.is_Derivative\
- or arg.is_Integer or arg.is_NumberSymbol\
- or arg.is_number\
- or arg.is_complex or arg.is_integer\
- or arg.is_real:
- return arg.conjugate()
- else:
- return None
- else:
- return None
- else:
- return d
-
- def _eval_dagger(self):
- return self.args[0]
-
@ellisonbg
SymPy member

Does the new adjoint class have all of these printers? They need to be tested in the IPython notebook and qtconsole to make sure they are working properly with ipython.

@jrioux
SymPy member
jrioux added a note Mar 28, 2012

I didn't copy sympyrepr and sympystr since they are the same as __str and repr. But _pretty and _latex were copied verbatim from Dagger to adjoint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ellisonbg ellisonbg and 1 other commented on an outdated diff Mar 27, 2012
sympy/physics/quantum/dagger.py
+adjoint.__name__ = "Dagger"
@ellisonbg
SymPy member

What does this line do exactly?

@jrioux
SymPy member
jrioux added a note Mar 28, 2012

This is here so that Dagger(x) prints as "Dagger(x)" and not "adjoint(x)".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jrioux
SymPy member

Passes doctests and tests on 32- and 64-bit python 2.7.0. Please see if you like it. By the way you can now have anti-Hermitian operators with Operator('A', antihermitian=True) which might be useful.

@asmeurer
SymPy member

This needs to be rebased/merged with master.

@asmeurer
SymPy member

SymPy Bot Summary: There were merge conflicts; could not test the branch.

@jrioux: Please rebase or merge your branch with master. See the report for a list of the merge conflicts.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY7cYSDA

Interpreter: /usr/local/bin/python2.5 (2.5.0-final-0)
Architecture: Darwin (32-bit)
Cache: yes
Test command: setup.py test
master hash: 909c079
branch hash: bda6b96b3adb2a6afe7222fe4e6e3d565f523dcd

Automatic review by SymPy Bot.

@jrioux
SymPy member

Rebased.

@ellisonbg
SymPy member

OK this looks good. I only have one question. Upon thinking further, I am having doubts on having a custom Dagger class in quantum. How do others think about 1) moving the extra logic that is in the quantum Dagger subclass to the base adjoint class (including its tests) and 2) getting rid of the Dagger module in quantum and 3) changing the quantum API so that users must use adjoint rather than dagger. Because Dagger is a commonly used class in quantum this is a semi disruptive change.

Try to deprecate the old Dagger module gracefully?

@lazovich, @flacjacket, @asmeurer, @certik what do you think of this?

@flacjacket
SymPy member

@ellisonbg I agree the logic for Dagger should be moved out of the quantum module. It may be nice to keep Dagger around as an alias for the new adjoint class, but if we choose to deprecate it, it should be fairly easy, like the current is_Real to is_Float deprecation, since the adjoint should behave like the current Dagger class in any cases where it would be used.

jrioux added some commits Mar 22, 2012
@jrioux jrioux Remove glob imports from sympy/matrices. 72ef458
@jrioux jrioux Replace 1/2 by S(1)/2 in sympy/physics/quantum/tests/test_spin.py abe3655
@jrioux jrioux Implement (anti-)Hermitian assumption for symbols.
In sympy, conjugate() is complex conjugation, not Hermite conjugation.
Implement adjoint() for Hermite conjugation and add an Hermitian
assumption for self-adjoint symbols.  Also implement anti-Hermitian
or skew-Hermitian assumption (anti-Hermitian implies not Hermitian).

>>> A = Symbol('A', hermitian=True)
>>> B = Symbol('B', antihermitian=True)
>>> adjoint(A)
A
>>> adjoint(B)
-B
>>> adjoint(A*B)
-A*B
>>> adjoint(B*B)
B**2

The adjoint() function is defined in the follow-up commit.
4c0ac8a
@jrioux jrioux Add is_commutative test for Dagger(). c8c595c
@jrioux jrioux Implement adjoint(): Hermite conjugation.
- adjoint():   Hermite conjugation or conjugate transposition,
- transpose(): Linear map transposition.

>>> A = Symbol('A', hermitian=True)
>>> B = Symbol('B', antihermitian=True)
>>> C = Symbol('C')
>>> M = Matrix(3, 1, [1,I,3])
>>> adjoint(A)
A
>>> adjoint(B)
-B
>>> adjoint(A*B)
-A*B
>>> adjoint(B*B)
B**2
>>> adjoint(C)
conjugate(transpose(C))
>>> M.adjoint()
[1, -I, 3]
04a297f
@jrioux jrioux Merge adjoint and Dagger implementations.
Use the printing that was written for Dagger in adjoint.

Let Dagger subclass adjoint and call adjoint() on its argument,
while using conjugate(transpose(argument)) as a fallback.

Implement _eval_adjoint for QExpr and all derived classes.
ccd490b
@jrioux jrioux Make physics/quantum rely on assumptions whenever possible. 86a12e4
@jrioux jrioux Implement conjugate() and adjoint() for MatMul. bd6df91
@jrioux
SymPy member

Rebased. The Dagger class is reduced to a minimum. The reason it cannot be further removed, is that adjoint() calls sympify on its input (this is consistent with conjugate()), while Dagger() does not. Since you cannot sympify a Matrix, adjoint(Matrix) returns a error and Matrix.adjoint() must be used instead. On the other hand Dagger(Matrix) works as expected, the behavior of the Dagger class is unchanged, and the code for it has been reduced to a minimum. I'd like to get this in, and have further merging of adjoint() with Dagger() considered a separate issue, since it involves to first implement Matrix sympification.

@jrioux
SymPy member

SymPy Bot Summary: All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY9eoTDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 56a8cb7
branch hash: bd6df91

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYwKwTDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 56a8cb7
branch hash: bd6df91

Automatic review by SymPy Bot.

@ellisonbg
SymPy member
@jrioux
SymPy member

See http://code.google.com/p/sympy/issues/detail?id=2244 ... we can special-case Matrix, but as @asmeurer said it is not very clean. Here we need to special-case numpy and scipy matrices too, to retain the current behavior of Dagger(). I'm not convinced if it's really the way to go. On the other hand most tests and the docstring could be moved, so if that is desirable I will go ahead and do that.

@ellisonbg
SymPy member

OK if it doesn't look clean that let's not try to put that logic in adjoint. Is there anything else to do on this PR?

@jrioux
SymPy member

I think it's ready to go.

@jrioux
SymPy member

SymPy Bot Summary: All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYn6gXDA

Interpreter: /usr/bin/python (2.7.2-final-0)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 8426fe8
branch hash: bd6df91

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYyfEWDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 8426fe8
branch hash: bd6df91

Automatic review by SymPy Bot.

jrioux added some commits May 2, 2012
@jrioux jrioux Merge branch 'master' into Hermitian
Conflicts:
	sympy/core/add.py
	sympy/functions/elementary/complexes.py
6a29e40
@jrioux jrioux Merge branch 'master' into Hermitian
Conflicts:
	sympy/core/assumptions.py
	sympy/functions/__init__.py
	sympy/physics/quantum/anticommutator.py
	sympy/physics/quantum/commutator.py
	sympy/physics/quantum/dagger.py
	sympy/physics/quantum/innerproduct.py
	sympy/physics/quantum/operator.py
	sympy/physics/quantum/qexpr.py
	sympy/physics/quantum/qft.py
	sympy/physics/quantum/tensorproduct.py
	sympy/physics/quantum/tests/test_spin.py
b799005
@asmeurer
SymPy member

SymPy Bot Summary: There were merge conflicts; could not test the branch.

@jrioux: Please rebase or merge your branch with master. See the report for a list of the merge conflicts.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYhtYaDA

Interpreter: /Library/Frameworks/Python.framework/Versions/3.2/bin/python3 (3.2.2-final-0)
Architecture: Darwin (64-bit)
Cache: yes
Test command: ./bin/test && ./bin/doctest
master hash: 53af081
branch hash: b799005

Automatic review by SymPy Bot.

@jrioux jrioux Merge branch 'master' into Hermitian
Conflicts:
	sympy/physics/quantum/dagger.py
ed71266
@jrioux
SymPy member

OK I merged it yet again.

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYz7sbDA

Interpreter: /usr/bin/python (2.7.2-final-0)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: f9d61ba
branch hash: ed71266

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY5YEcDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: f9d61ba
branch hash: ed71266

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@jrioux: Please fix the test failures.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYm8UdDA

Interpreter: /usr/bin/python (2.7.2-final-0)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 6925f5e
branch hash: ed71266

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY_cwdDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 6925f5e
branch hash: ed71266

Automatic review by SymPy Bot.

@jrioux jrioux Merge branch 'master' into Hermitian
Conflicts:
	sympy/matrices/__init__.py
	sympy/matrices/expressions/__init__.py
e00edb7
@travisbot

This pull request passes (merged e00edb7 into 48f42f8).

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY__AeDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 48f42f8
branch hash: e00edb7

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYuOwdDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 424160a
branch hash: e00edb7

Automatic review by SymPy Bot.

@mrocklin
SymPy member

Was just referred to this PR. If you want to solve the sympify(Matrix) problem I suggest you use ImmutableMatrix.

@jrioux jrioux Merge branch 'master' into Hermitian
Conflicts:
	sympy/core/add.py
	sympy/core/power.py
	sympy/matrices/expressions/blockmatrix.py
	sympy/matrices/expressions/matexpr.py
	sympy/matrices/expressions/transpose.py
	sympy/matrices/immutable_matrix.py
85a2286
@travisbot

This pull request fails (merged 85a2286 into 3c2c3c7).

@jrioux
SymPy member

Not sure why it failed to pull the branch.

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYgfUiDA

Interpreter: /usr/bin/python (2.7.0-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 0d88b25
branch hash: 85a2286

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYi8YiDA

Interpreter: /usr/bin/python (2.7.2-final-0)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 0d88b25
branch hash: 85a2286

Automatic review by SymPy Bot.

@certik certik commented on the diff Aug 10, 2012
sympy/assumptions/ask.py
Equivalent(Q.even, Q.integer & ~Q.odd),
Equivalent(Q.extended_real, Q.real | Q.infinity),
Equivalent(Q.odd, Q.integer & ~Q.even),
Equivalent(Q.prime, Q.integer & Q.positive & ~Q.composite),
Implies (Q.integer, Q.rational),
Implies (Q.imaginary, Q.complex & ~Q.real),
+ Implies (Q.imaginary, Q.antihermitian),
+ Implies (Q.antihermitian, ~Q.hermitian),
@certik
SymPy member
certik added a note Aug 10, 2012

Shouldn't these relations be tested? It seems to me the only test for this PR is the one line at the very end:

assert Dagger(A).is_commutative == False

so this PR should be much more tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@certik
SymPy member

So besides more testing, it looks ok to me.

Can you also please rebase it on top of the master? It doesn't apply cleanly.

jrioux added some commits Aug 11, 2012
@jrioux jrioux Merge branch 'master' into Hermitian
Conflicts:
	sympy/matrices/__init__.py
	sympy/matrices/expressions/__init__.py
	sympy/matrices/expressions/blockmatrix.py
	sympy/matrices/immutable_matrix.py
faaee09
@jrioux jrioux Correctly check for noncommutativity instead of commutativity. 9c1a556
@jrioux jrioux Test the assumption handlers for (anti-)Hermitian. dff47ab
@jrioux
SymPy member

I added tests and corrected a small mistake along the way.

@travisbot

This pull request passes (merged dff47ab into 3b77c94).

@Krastanov
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

Test command: setup.py test
master hash: b0a9866
branch hash: dff47ab

Interpreter 1: ✳️ All tests have passed.

Interpreter: /usr/local/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY1qsjDA

Interpreter 2: ✳️ All tests have passed.

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYpJQjDA

Interpreter 3: ✳️ All tests have passed.

Interpreter: /usr/bin/python3.2 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY1N0iDA

Build HTML Docs: ✳️ All tests have passed.

Docs build command: make html-errors
Sphinx version: 1.1.3

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sY3rYiDA

Automatic review by SymPy Bot.

@certik
SymPy member

I think this is awesome. All tests pass, so I am merging this. We can improve upon this once this is in master.

@certik certik merged commit 25eab6a into sympy:master Aug 13, 2012

1 check passed

Details default The Travis build passed
@jrioux
SymPy member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment