Skip to content

Use the correct decoherence correction by default#104

Merged
danielhollas merged 2 commits intomasterfrom
correct-decoherence
Apr 27, 2022
Merged

Use the correct decoherence correction by default#104
danielhollas merged 2 commits intomasterfrom
correct-decoherence

Conversation

@danielhollas
Copy link
Copy Markdown
Contributor

Due to the error in the equation in the original paper, the decoherence correction was not implemented correctly
(never take anything for granted!).

This was already fixed in commit c7b342b, here we switch the use of correct formula by default.

To reproduce old results, add correct_decoherence=.false in the input file in namelist sh.

Thanks to Basile Curchod and Antonio Prlj for catching this!

Due to the error in the equation in the original paper,
the decoherence correction was not implemented correctly
(never take anything for granted!).
This was already fixed in commit c7b342b,
here we switch the use of correct formula by default.

To reproduce old results, add correct_decoherence=.false.
in the input file in namelist sh.

Thanks to Basile Curchod and Antonio Prlj for catching this!
@danielhollas danielhollas self-assigned this Apr 1, 2022
@danielhollas danielhollas requested a review from suchanj April 1, 2022 15:57
@danielhollas
Copy link
Copy Markdown
Contributor Author

@suchanj I believe you did a bit of testing on this and found that it didn't have a huge effect, right?

Once I am done with all the refactor, I will test the SH code thoroughly again against the molecular Tully model molecules from Basile.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 1, 2022

Codecov Report

Merging #104 (d327c40) into master (2695354) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #104   +/-   ##
=======================================
  Coverage   91.73%   91.73%           
=======================================
  Files          42       42           
  Lines        5829     5829           
=======================================
  Hits         5347     5347           
  Misses        482      482           
Flag Coverage Δ
unittests 23.11% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/sh_integ.F90 97.02% <ø> (ø)

@suchanj
Copy link
Copy Markdown
Contributor

suchanj commented Apr 1, 2022

Yes, the limited testing proved no major deviations between new and old decoherence treatment.

@danielhollas
Copy link
Copy Markdown
Contributor Author

danielhollas commented Apr 27, 2022

@suchanj FYI I am merging this to master for later testing, hopefully by the end of this week or next week.

@danielhollas danielhollas merged commit 397f09d into master Apr 27, 2022
@danielhollas danielhollas deleted the correct-decoherence branch April 27, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants