Tr printing, Tr partial trace #1396

Merged
merged 14 commits into from Jul 16, 2012

10 participants

@gdevanla

This PR implements printing operations for Tr operators and partial trace operations. Also cycle permutes are fixed for consistency. Some changes added to Density class to enable partial trace operations. Also, new set of Notebook examples have been provided.

@ellisonbg , @flacjacket, could you please review the design.

@gdevanla

The printing currently does not print out the indices that are provided as part of object creation. We need to figure out a way to output this along with all other arguments to Tr. Should it just be a [list] displayed after all other arguments to Tr or some super-script/subscript?

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /opt/pym32/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: d136cb7
branch hash: 11bb365255ed727e8bbea2e192c2a4c7235917a8

Automatic review by SymPy Bot.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: d136cb7
branch hash: 11bb365255ed727e8bbea2e192c2a4c7235917a8

Automatic review by SymPy Bot.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python3 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: d136cb7
branch hash: 11bb365255ed727e8bbea2e192c2a4c7235917a8

Automatic review by SymPy Bot.

@travisbot

This pull request fails (merged 11bb3652 into d136cb7).

@travisbot

This pull request fails (merged ed384052 into d136cb7).

@travisbot

This pull request fails (merged 88eed960 into d136cb7).

@certik
SymPy member

@gdevanla, there are failures caused by your pull request:

________________________________________________________________________________
2575____________ sympy/physics/quantum/tests/test_density.py:test_doit _____________
2576  File "/home/vagrant/virtualenv/python2.5/lib/python2.5/site-packages/sympy/physics/quantum/tests/test_density.py", line 49, in test_doit
2577    assert d.doit() == (1.0*A*C*Dagger(C)*Dagger(A) +
2578  File "/home/vagrant/virtualenv/python2.5/lib/python2.5/site-packages/sympy/physics/quantum/density.py", line 167, in doit
2579    for arg in itertools.product(state.args, repeat=2):
2580AttributeError: 'module' object has no attribute 'product'
2581________________________________________________________________________________
2582______ sympy/physics/quantum/tests/test_tensorproduct.py:test_eval_trace _______
2583  File "/home/vagrant/virtualenv/python2.5/lib/python2.5/site-packages/sympy/physics/quantum/tests/test_tensorproduct.py", line 94, in test_eval_trace
2584    assert t.doit() == ( 1.0*Tr(A*Dagger(A))*Tr(B*Dagger(B)) +
2585  File "/home/vagrant/virtualenv/python2.5/lib/python2.5/site-packages/sympy/core/trace.py", line 146, in doit
2586    return self.args[0]._eval_trace(indices=self.args[1])
2587  File "/home/vagrant/virtualenv/python2.5/lib/python2.5/site-packages/sympy/physics/quantum/density.py", line 193, in _eval_trace
2588    return Tr(self.doit(), indices).doit()
2589  File "/home/vagrant/virtualenv/python2.5/lib/python2.5/site-packages/sympy/physics/quantum/density.py", line 167, in doit
2590    for arg in itertools.product(state.args, repeat=2):
2591AttributeError: 'module' object has no attribute 'product'

Only the last failure was already in master, and the pull request #1398 fixes that one.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /opt/pym32/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 9ba3cb3
branch hash: 88eed960ae243f780460bfa9d3bccb7cad83260f

Automatic review by SymPy Bot.

@gdevanla

Ok, the error message is since itertools.product is not supported in Python 2.5 Will fix that soon.

@asmeurer
SymPy member

There's probably a replacement in the compatibility file.

@asmeurer
SymPy member

I'm curious why the trace stuff is in the core.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 9ba3cb3
branch hash: 88eed960ae243f780460bfa9d3bccb7cad83260f

Automatic review by SymPy Bot.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python3 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 0930232
branch hash: 88eed960ae243f780460bfa9d3bccb7cad83260f

Automatic review by SymPy Bot.

@gdevanla

@asmeurer The Tr module implemented currently is generic and does not have to belong to the physics module. The module in turn calls _eval_trace() methods in objects passed as 'args' to Tr. For now, yes _eval_trace() methods are defined for objects under physics/quantum module, but that need not be true always.

@travisbot

This pull request passes (merged b01f7e5 into 0930232).

@Krastanov
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

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

Interpreter: /opt/pym32/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 0930232
branch hash: b01f7e5

Automatic review by SymPy Bot.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 0930232
branch hash: b01f7e5

Automatic review by SymPy Bot.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python3 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 0930232
branch hash: b01f7e5

Automatic review by SymPy Bot.

@asmeurer
SymPy member

I agree it is general enough to not be in the physics module, but that doesn't mean it belongs in the core. Only very basic stuff that is used by everything should be in the core. Trace I think belongs somewhere else.

@gdevanla

Yes, one could say it is not a core operation. Do you have any suggestion on where it should go?

@ellisonbg Do you have any suggestion, based how the Tr operator would be used in the future?

@flacjacket
SymPy member

functions.elementary might be a good place for it.

@asmeurer
SymPy member

I'd put it somewhere other than elementary. That's supposed to be only elementary functions. See http://code.google.com/p/sympy/issues/detail?id=2149.

@flacjacket flacjacket commented on the diff Jul 6, 2012
examples/notebooks/density.ipynb
+ "source": [
+ "## Density operators with Tensor Products as args"
+ ]
+ },
+ {
+ "cell_type": "code",
+ "collapsed": false,
+ "input": [
+ "from sympy.core.trace import Tr",
+ "",
+ "A, B, C, D = symbols('A B C D',commutative=False)",
+ "",
+ "t1 = TensorProduct(A,B,C)",
+ "",
+ "d = Density([t1, 1.0])",
+ "d.doit()",
@flacjacket
SymPy member
flacjacket added a line comment Jul 6, 2012

To display this result, you'll either need to split the cell here or use display() from IPython.core.display

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

The rest of the changes seem alright to me, once we figure out a better place for trace to go.

@gdevanla

I have other changes siting in the follow up branch. I will move the discussion on where Tr should go to the mailing list. I can submit a separate PR with just the re-factored changes. Will that work?

Alternatively, the one place I can think of after looking at existing folder structures is at : sympy/matrices/expressions. This folder already has modules for matadd, matmul, matpow, inverse and transpose.

@travisbot

This pull request passes (merged e1a3a01 into 0930232).

@Krastanov
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

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

Interpreter: /opt/pym32/bin/python2.5 (2.5.6-final-0)
Architecture: Linux (32-bit)
Cache: yes
Test command: setup.py test
master hash: 424160a
branch hash: e1a3a01

Automatic review by SymPy Bot.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python2.7 (2.7.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 424160a
branch hash: e1a3a01

Automatic review by SymPy Bot.

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

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

Interpreter: /usr/bin/python3 (3.2.3-candidate-2)
Architecture: Linux (64-bit)
Cache: yes
Test command: setup.py test
master hash: 424160a
branch hash: e1a3a01

Automatic review by SymPy Bot.

@flacjacket
SymPy member

Guru, have you sent a message to the list? I feel like since the damage was done and trace.py was put in core in the last PR, it should be alright to merge this and fix it in a later PR when a good spot is found. @asmeurer: what do you think about that?

@ellisonbg have you taken a look at this yet?

@asmeurer
SymPy member

That should be fine.

@gdevanla

Ok, once @ellisonbg is fine with these changes, this can go.

I will start the mailing thread to discuss the where the Tr module should go.

@ellisonbg
SymPy member
@ellisonbg
SymPy member

I think this looks good. I will give some feedback on individual lines. My broad comment is that all your tests and notebook examples use symbols instead of bras/kets. Please include some tests with actual bras/kets and use bras/kets in the notebook examples.

@ellisonbg ellisonbg and 1 other commented on an outdated diff Jul 12, 2012
sympy/core/trace.py
expr = args[0]
- indices = args[1] if len(args) == 2 else -1 #-1 indicates full trace
+ indices = args[1] if len(args) == 2 else []
@ellisonbg
SymPy member
ellisonbg added a line comment Jul 12, 2012

@flacjacket do you think we should wrap indices here in a sympy Tuple?

@flacjacket
SymPy member
flacjacket added a line comment Jul 12, 2012

Yes, we should. I'm not sure why this doesn't trip the test_args test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ellisonbg ellisonbg commented on the diff Jul 12, 2012
sympy/core/trace.py
return True
+
+
+ #TODO: Review if the permute method is needed
+ # and if it needs to return a new instance
+ #def permute(self, pos):
@ellisonbg
SymPy member
ellisonbg added a line comment Jul 12, 2012

I would like to have the permute method and it should return a new instance.

@gdevanla
gdevanla added a line comment Jul 13, 2012

@ellisonbg Actually, I had started a discussion on this on the mailing list. http://bit.ly/M7D9wF

I can work on this and see how we can achieve this. I will add to my TODO. In the subsequent PR, I will update this method based on how we decide to do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ellisonbg ellisonbg commented on the diff Jul 12, 2012
sympy/physics/quantum/density.py
terms = []
for (state, prob) in self.args:
- terms.append(prob*(state*Dagger(state)))
+ state = state.expand() # needed to break up (a+b)*c
+ if (isinstance(state, Add)):
+ for arg in product(state.args, repeat=2):
+ terms.append(prob *
+ self._generate_outer_prod(arg[0], arg[1]))
@ellisonbg
SymPy member
ellisonbg added a line comment Jul 12, 2012

The _generate_outer_product method should be called _generate_tensor_product to be consistent with our naming. Also, can you summarize what this method does and why it is needed?

@gdevanla
gdevanla added a line comment Jul 13, 2012

@ellisonbg I am guessing you were commenting on the _generate_outer_prod in tensor_product. I plan to remove that function.

The function here is just to reuse code needed within the for and if constructs. Am I in sync?

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 Jul 12, 2012
sympy/physics/quantum/tensorproduct.py
@@ -196,6 +197,18 @@ def expand(self, **hints):
tp = TensorProduct(*[sympify(item).expand(**hints) for item in self.args])
return Expr.expand(tp, **hints)
+ def _generate_outer_prod(self,arg):
@ellisonbg
SymPy member
ellisonbg added a line comment Jul 12, 2012

Minimally we need docs to describe why this is needed. Why can we just call tensor_product_simp directly?

@gdevanla
gdevanla added a line comment Jul 13, 2012

Agreed, the redirection is not needed. I will invoke tensor_product_simp directly.

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

This can come in a later PR, but I would like to see some tests with Traces of density matrices of tensor products of spin states to make sure the logic works when the representations are matrices.

@travisbot

This pull request fails (merged bcd53e1 into 0930232).

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

Test command: setup.py test
master hash: 387547b
branch hash: bcd53e1

Interpreter 1: 🔴 There were test failures.

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/agZzeW1weTNyDAsSBFRhc2sYya8fDA

Interpreter 2: 🔴 There were test failures.

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/agZzeW1weTNyDAsSBFRhc2sY8ZwgDA

Interpreter 3: 🔴 There were test failures.

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/agZzeW1weTNyDAsSBFRhc2sY7vUfDA

Build HTML Docs: 🔴 There were test failures.

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

Test results html report: http://reviews.sympy.org/report/agZzeW1weTNyDAsSBFRhc2sYm-YfDA

Automatic review by SymPy Bot.

@travisbot

This pull request fails (merged 4f3405d into 0930232).

@Krastanov
SymPy member

SymPy Bot Summary: 🔴 There were test failures.

@gdevanla: Please fix the test failures.

Test command: setup.py test
master hash: 0abed0f
branch hash: 4f3405d

Interpreter 1: 🔴 There were test failures.

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/agZzeW1weTNyDAsSBFRhc2sYn-YfDA

Interpreter 2: 🔴 There were test failures.

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/agZzeW1weTNyDAsSBFRhc2sYk_EeDA

Interpreter 3: 🔴 There were test failures.

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/agZzeW1weTNyDAsSBFRhc2sYkJUgDA

Build HTML Docs: 🔴 There were test failures.

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

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

Automatic review by SymPy Bot.

@jrioux
SymPy member

SymPy Bot Summary: ✳️ All tests have passed.

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

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

Automatic review by SymPy Bot.

@ellisonbg
SymPy member

This is ready for merge, and the latest test run seems to pass. But the previous one had lots of failures that seem unrelated to this PR. Do anyone understand those failures?

@jrioux
SymPy member

There was a failure in test_gruntz in the master branch, which has since been fixed. There are still failures related to hash randomization in master, which makes it hard to evaluate whether any failure is introduced by this pull request or not. Judging by the latest test results here, I would say it is safe to merge this pull request.

@ellisonbg ellisonbg merged commit b33c1ed into sympy:master Jul 16, 2012
@amakelov

I'm not sure if this is relevant, but a recent run of travisbot on one of my PRs brought up an error in test_density.py. Here's the pull request: #1406 and the error report: http://travis-ci.org/#!/sympy/sympy/builds/1879684

@jrioux jrioux commented on the diff Jul 16, 2012
sympy/physics/quantum/tests/test_density.py
+
+ d = Density([t2, 0.5], [t3, 0.5])
+ assert d.doit() == (0.5 * TensorProduct(A*Dagger(A), B*Dagger(B)) +
+ 0.5 * TensorProduct(C*Dagger(C), D*Dagger(D)))
+
+ #Density with mixed states
+ d = Density([t2+t3,1.0])
+ assert d.doit() == (1.0 * TensorProduct(A*Dagger(A), B*Dagger(B)) +
+ 1.0 * TensorProduct(A*Dagger(C), B*Dagger(D)) +
+ 1.0 * TensorProduct(C*Dagger(A), D*Dagger(B)) +
+ 1.0 * TensorProduct(C*Dagger(C), D*Dagger(D)))
+
+
+ #Density operators with spin states
+ tp1 = TensorProduct(JzKet(1,1), JzKet(1,-1))
+ tp2 = TensorProduct(JzKet(1,-1/2), JzKet(1,-1/2))
@jrioux
SymPy member
jrioux added a line comment Jul 16, 2012

To avoid the python 3 problem this could be written tp2 = TensorProduct(JzKet(1,-S(1)/2), JzKet(1,-S(1)/2))

@asmeurer
SymPy member
asmeurer added a line comment Jul 16, 2012

Note that in Python 2, this is giving 0, because you aren't using future division. Try adding from __future__ import division to the top of file.

I guess the question is if you want this to work with floating point half numbers.

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

The problem (according to the error message) is that j and m are not both integer (or both half-integer). The message could be rewritten from "Both j and m must be integer or half-integer" to "j and m must both be integer or both be half-integer"

@gdevanla

I submitted a follow-up pull request to address this issue at #1425

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