search

tmancal74 / quantarhei

Open Quantum System Theory for Molecular Systems
MIT License
21 stars 16 forks source link

Superoperators #91

Closed dkrasa closed 6 years ago

dkrasa commented 6 years ago

Correct relations between (at least some) super operators were added. RelaxationTensor now inherits from SuperOperator.

tmancal74 commented 6 years ago

The problem seems to be in different precision of output in printing the transition energies and dipole moments in exciton_report() of Aggregate. Please, set the precision of the output in Aggregate and update tests.

dkrasa commented 6 years ago

I have fixed the exciton_report() formatting and its tests. Local tests run well.

tmancal74 commented 6 years ago

Still some problems with formatting. This time it is LabSetup. We print a result of DFunction at() method, which is a numpy array (in this case). Try to check the numpy.set_printoptions call in the quantarhei.core.managers.py on line 129. Maybe including the legacy option will fix the output again.

dkrasa commented 6 years ago

My local tests now fail somewhere, where they did not before, but the previous problem seems to be fixed. I need to see response from the CI server.

codecov[bot] commented 6 years ago

Codecov Report

Merging #91 into master will decrease coverage by 0.05%. The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #91      +/-   ##
==========================================
- Coverage   63.99%   63.94%   -0.06%     
==========================================
  Files         140      140              
  Lines       14774    14817      +43     
==========================================
+ Hits         9454     9474      +20     
- Misses       5320     5343      +23
Impacted Files Coverage Δ
quantarhei/spectroscopy/labsetup.py 100% <ø> (ø) :arrow_up:
quantarhei/qm/hilbertspace/hamiltonian.py 66.32% <100%> (ø) :arrow_up:
quantarhei/core/managers.py 71.29% <100%> (-0.48%) :arrow_down:
quantarhei/qm/liouvillespace/redfieldfoerster.py 23.43% <100%> (+1.21%) :arrow_up:
quantarhei/builders/aggregate_excitonanalysis.py 93.67% <100%> (ø) :arrow_up:
quantarhei/qm/liouvillespace/redfieldtensor.py 82.11% <100%> (+0.11%) :arrow_up:
quantarhei/qm/liouvillespace/relaxationtensor.py 71.27% <50%> (-4.26%) :arrow_down:
quantarhei/qm/liouvillespace/foerstertensor.py 92.42% <66.66%> (-1.33%) :arrow_down:
...tarhei/qm/liouvillespace/evolutionsuperoperator.py 73.46% <73.33%> (-0.95%) :arrow_down:
quantarhei/qm/liouvillespace/superoperator.py 84.9% <76.66%> (-11.25%) :arrow_down:
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a5f8309...0e2ff6c. Read the comment docs.

dkrasa commented 6 years ago

It passed on Travis-CI, but it seems that their version of numpy does not behave as it should. It seems not to react to the legacy='1.13' setting which should suppress the + sign in the complex number representation. I will try to ask trevis to install numpy 1.14.0 to get rid of the legacy problems

dkrasa commented 6 years ago

Now I removed the troubled test altogether. It was not necessary at that point at all.

tmancal74 commented 6 years ago

I accept the pull request. I take it as a message from this discussion that we should go over all str methods defined in Quantarhei and thoroughly test their output and make sure that they give consistent representation.