Moving the coordination number to mctc-lib#71
Conversation
awvwgk
left a comment
awvwgk
left a comment
There was a problem hiding this comment.
The update for the GH actions can be a separate PR.
|
Sure. I moved the update of the GH actions to (#72). I am mostly interested if you think that mctc-lib is the right place for the coordination numbers. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 69.79% 69.06% -0.73%
==========================================
Files 64 79 +15
Lines 8618 9957 +1339
Branches 2579 3150 +571
==========================================
+ Hits 6015 6877 +862
- Misses 782 848 +66
- Partials 1821 2232 +411 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I think this is now ready. I opened draft PRs for the changes necessary in dftd4, simple-dftd3, multicharge, and tblite. Currently, they point to this branch and will be updated to the mctc-lib head as soon as this PR is merged. This PR collects all coordination numbers with different counting functions, electronegativity dependences, and cutoffs in mctc-lib. They are used via a coordination number type. Additionally, common data such as Pauling electronegativity or different radii are collected in mctc-lib. |
4 participants
It would be quite beneficial to have a centralized place to calculate the CN since it appears in all of our projects (xtb, tblite, dftd4, dftd3, multicharge, DRACO, ...). Moreover, there are now several different CNs based on slight variations of the counting function, which must be integrated in several places.
mctc-libmight be the right spot since it is purely based on the structure, but I am open to alternatives (@awvwgk, @Albkat, @marvinfriede). As an example, I moved the tblite implementation to the mctc-lib. This also includes the cutoffs for periodicity and part of the data sections (maybe we should move the whole as literature radii and EN are also frequently used).If we should do this, I will still clean up a few things in the CN (i.e. rename the gfn CN to dexp for double exponential) to make it general.