-
Notifications
You must be signed in to change notification settings - Fork 20
Moving the coordination number to mctc-lib #71
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
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-lib
might 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.