Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Projection of fixed atoms out of Hessian in L-ANCOPT and FIRE optimizers #433

Merged
merged 2 commits into from
Feb 2, 2021

Conversation

clavigne
Copy link
Contributor

@clavigne clavigne commented Feb 2, 2021

This PR fixes #410.

I've tested that it works using this xcontrol file for FIRE and this one for L-ANCOPT, with this initial geometry.

Running L-ANCOPT prior to the fix yields this final geometry

3
xtb: 6.4.0 (d4b70c2b361067f291e81c3df1d1c31c28d01cb4)
H        -0.79575145519344    0.12524065205080   -0.00000000099274
H         0.96541707812111    0.14678380837277   -0.00000000237865
O         0.03033437297265    0.72797553581838    0.00000000000000

After this PR, running the same optimization yields

3
xtb: 6.4.0 (d4b70c2b361067f291e81c3df1d1c31c28d01cb4)
H        -0.77226423724448    0.14366978476487    0.00000000477125
H         0.97226467309956    0.14366934440933   -0.00000000941527
O         0.01526020355893    0.62582966774369    0.00000000000000

which has the correct coordinates for the first first two atoms. For FIRE, the projection is only involved when the optimization is preconditioned, which is currently not a setable option. However, I hand-set it to obtain the following pre-PR optimization result

3
xtb: 6.4.0 (d4b70c2b361067f291e81c3df1d1c31c28d01cb4)
H        -0.63257492583150    0.05605455560180    0.00000000000000
H         1.20029965304500   -0.14298093394565   -0.00000000000000
O         0.33157878141598    0.39598095953997    0.00000000000000

and post-PR,

3
xtb: 6.4.0 (d4b70c2b361067f291e81c3df1d1c31c28d01cb4)
H        -0.77226423661793    0.14366978520871   -0.00000000000000
H         0.97226467499076    0.14366934683588   -0.00000000000000
O         0.10047054653320    0.62475919881618   -0.00000000000000

The important part is that in either cases the pre-PR includes significant displacement of the fixed atoms, due to the lack of Hessian projection. This is now fixed.

Cyrille Lavigne added 2 commits February 1, 2021 19:01
This only matters when preconditioning is turned on, which is currently set
explicitly to false, with no user-adjustable settings to turn it on.

Signed-off-by: Cyrille Lavigne <[email protected]>
@codecov
Copy link

codecov bot commented Feb 2, 2021

Codecov Report

Merging #433 (8752196) into master (d4b70c2) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #433      +/-   ##
==========================================
- Coverage   40.80%   40.79%   -0.02%     
==========================================
  Files         304      304              
  Lines       50774    50787      +13     
==========================================
  Hits        20718    20718              
- Misses      30056    30069      +13     
Impacted Files Coverage Δ
src/relaxation_engine.f90 0.00% <0.00%> (ø)

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 d4b70c2...bd64341. Read the comment docs.

Copy link
Member

@awvwgk awvwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sharing.

@awvwgk awvwgk added this to the v6.4.1 milestone Feb 2, 2021
@awvwgk awvwgk merged commit e1991fd into grimme-lab:master Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hessian is not correctly projected for LBFGS/L-ANCopt
2 participants