dftbplus / skprogs

Basic programs for generating Slater-Koster files for the DFTB-method
GNU Lesser General Public License v3.0
25 stars 19 forks source link

Woods-Saxon confinement #90

Open vanderhe opened 3 months ago

vanderhe commented 3 months ago

Implements the Woods-Saxon confinement potential.

Closes #89.

vanderhe commented 3 months ago

@samtsevich I have generalized parts of the infrastructure. To be continued here:

https://github.com/vanderhe/skprogs/blob/d27e6b355a1f526cef853422f9e7a355998bd96c/slateratom/prog/main.f90#L94-L101

vanderhe commented 3 months ago

@samtsevich I have switched over to an all numeric integration of the confinement potential and abstracted out the potentials form. In the future we could now easily add 'arbitrary' potentials without involved modifications. Validation is still pending, but I'm mostly confident that it works as intended, as the results of the previous analytical power compression are reproduced, i.e. the regression tests pass.

vanderhe commented 3 months ago

C_veff.pdf

vanderhe commented 3 months ago

@samtsevich I think this is fully operational now. Any ideas on how to further validate the newly implemented WS compression?

chiarapan commented 3 months ago

Wow that was fast, thank you so much! Not sure if we can validate it e.g. against published parametrizations (Chou's paper, or my lithium-graphite) because the atomic solvers and probably other finer details are all different and/or not available. Did you already give it a test run with some actual SK table generation?

vanderhe commented 3 months ago

Did you already give it a test run with some actual SK table generation?

I have generated some SK-files but did not perform the actual DFTB calculation. The two-center integration does not explicitly depend on the confinement potential, so I guess it is fine. The new numeric approach to the confinement in the one-center part seems to perfectly reproduce the analytical power compression and the WS implementation follows the same code path (but of course with a different functional form). You have way more experience with the WS compression than I have, would it be possible that you briefly check for a 'qualitatively correct/expected behavior'? But it is not urgent (at least not from my side).

chiarapan commented 3 months ago

I'll check soon with lithium (for which I haven't yet managed to get a great parametrization with the power compression). I'll update as soon as I test it!

vanderhe commented 3 months ago

I'll check soon with lithium (for which I haven't yet managed to get a great parametrization with the power compression). I'll update as soon as I test it!

Thanks a lot :+1:.

samtsevich commented 3 months ago

@vanderhe Hey Tammo, Should we first test this implementation before it is merged into the main repo or is that on your side? [Nevertheless, we can start playing even with this version, and lets see how that works for our optimization procedure]

chiarapan commented 3 months ago

I'll check soon with lithium (for which I haven't yet managed to get a great parametrization with the power compression). I'll update as soon as I test it!

Thanks a lot 👍.

Tested, it seems to work fine!

samtsevich commented 2 months ago

@aradi Can you please review these changes? On our side, we are ready to start playing with the optimization run

vanderhe commented 2 months ago

@aradi Can you please review these changes? On our side, we are ready to start playing with the optimization run

Bálint currently is still on vacation and will pick up work next week. I would estimate that a realistic time frame for this PR getting reviewed/merged is 1-2 weeks from now. From my side this is the final version, so I would suggest that you resort to this branch until it gets merged into main.

vanderhe commented 2 months ago

@aradi Just a request: please do not squash f10a9b4 into any of the other commits when merging, I would like to keep the (fully working) $R\mathrm{onset}, R\mathrm{cut}$-representation in the history, if possible.

vanderhe commented 2 months ago

@aradi I had to push an additional commit to this branch that fixes a bug in the shelf search. Nothing dramatic, but the previous version was not recognizing matching calculations in the shelf anymore.