duartegroup / autodE

automated reaction profile generation
https://duartegroup.github.io/autodE/
MIT License
165 stars 51 forks source link

Add option to modify etemp for xTB calculations #246

Closed shoubhikraj closed 1 year ago

shoubhikraj commented 1 year ago

xTB always uses an electronic temperature for calculations (default 300K). Sometimes the SCC can be stabilised by using a higher electronic temperature. This PR adds the feature of changing the temperature for xTB though Config.XTB.etemp.


Checklist

shoubhikraj commented 1 year ago

@t-young31 I have not looked through the code for wrappers in autodE in great detail, could you please check that I have not broken anything in this PR? 😅

codecov[bot] commented 1 year ago

Codecov Report

Merging #246 (11c64dc) into v1.4.0 (0d03666) will increase coverage by 0.17%. The diff coverage is 96.29%.

:exclamation: Current head 11c64dc differs from pull request most recent head 5c48c8c. Consider uploading reports for the commit 5c48c8c to get more accurate results

@@            Coverage Diff             @@
##           v1.4.0     #246      +/-   ##
==========================================
+ Coverage   97.14%   97.32%   +0.17%     
==========================================
  Files         196      195       -1     
  Lines       20172    20364     +192     
==========================================
+ Hits        19597    19820     +223     
+ Misses        575      544      -31     
Flag Coverage Δ
unittests 97.32% <96.29%> (+0.17%) :arrow_up:

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

Impacted Files Coverage Δ
tests/test_conf_gen.py 100.00% <ø> (ø)
tests/test_methods.py 100.00% <ø> (ø)
tests/test_plotting.py 100.00% <ø> (ø)
tests/test_hessian.py 98.57% <83.33%> (ø)
autode/utils.py 95.45% <91.86%> (-0.59%) :arrow_down:
tests/test_utils.py 96.94% <97.84%> (+0.43%) :arrow_up:
autode/__init__.py 100.00% <100.00%> (ø)
autode/config.py 100.00% <100.00%> (ø)
autode/conformers/conformers.py 100.00% <100.00%> (ø)
autode/constraints.py 98.70% <100.00%> (ø)
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

t-young31 commented 1 year ago

Given this changes the default behaviour of XTB I think this should target v1.4.0. Also, would you mind adding a test that checks that XTB is actually called with an etemp flag? (i.e. just checking --etemp 300 is in the output)

shoubhikraj commented 1 year ago

@t-young31 Thanks, I have changed the base branch. (also, it should not change the behaviour of xTB as the default temperature for xTB is 300K). I will add the tests soon.

t-young31 commented 1 year ago

(also, it should not change the behaviour of xTB as the default temperature for xTB is 300K

oh sorry – I assumed it'd be 0 K. Feel free to go for v1.3.5 then, if you want 😄

shoubhikraj commented 1 year ago

@t-young31 Hi, I was wondering how I could test for the --etemp 300 in the output? This is added to the command line flags, like --uhf or --chrg, and then passed directly to the subprocess. I am not quite sure how to check if the flag is present. Apart from maybe checking the results of the xTB output and looking at the temperature. Otherwise, I would have to write the temperature into xcontrol file instead of command line and then test for that file. I would appreciate your input.

t-young31 commented 1 year ago

pretty sure xtb prints the flags in the output file

shoubhikraj commented 1 year ago

Closing this PR to open a new expanded PR #252