tmancal74 / quantarhei

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

The changes to b777 and renger model #106

Closed foxfoxfox7 closed 5 years ago

foxfoxfox7 commented 5 years ago

from ...core.frequency import FrequencyAxis

can be re-commented out again

There was some issue with using freq1/freq2 If I use them and convert from cm to int, the combined spectral density seems to have the wrong units. If i do not convert them (and rely on the init conversion), the initial spec density has the wrong units but the combined one looks right :S. It feels like the units are being converted a second time when the spec dens is recalculated for combining but maybe not. Either way, using om1/om2 instead of freq1/freq2 makes it all work fine.

cfce = (numpy.amax(j*cropped_omega2)/numpy.amax(cfce)) cfce numpy.piS0(1/(s[0] + s[1])) This factor is included originally. The cropped_omega2 is obviously just the part that converts from one type of spec dens to the other. It is now included in the Renger part. the rest is strange. I can find nothing about it in the literature or in some books. the numpy.amax(j*)/numpy.amax(cfce) is just 1 and the rest is a factor is ~1.2. But the spec densities are the same without this factor and the ~1.2 just changes the polynomial form away from the Renger one. Maybe we can just delete?

If we can then we can also delete S0 variable as it is only used here.

codecov[bot] commented 5 years ago

Codecov Report

Merging #106 into master will increase coverage by <.01%. The diff coverage is 9.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
+ Coverage   41.25%   41.25%   +<.01%     
==========================================
  Files         108      108              
  Lines       13825    13825              
==========================================
+ Hits         5703     5704       +1     
+ Misses       8122     8121       -1
Impacted Files Coverage Δ
quantarhei/qm/corfunctions/spectraldensities.py 10.33% <9.09%> (+0.33%) :arrow_up:

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 ac91a38...b6159ae. Read the comment docs.

tmancal74 commented 5 years ago

The standard solution to units conversion is to use the function of the Manager class. Most likely it will be used as

the_number_in_internal_units = self.convert_energy_2_internal_u(number_you_input)