Skip to content

Move coordination number and common data to mctc-lib#135

Merged
marvinfriede merged 10 commits intodftd3:mainfrom
thfroitzheim:ncoord
Apr 8, 2025
Merged

Move coordination number and common data to mctc-lib#135
marvinfriede merged 10 commits intodftd3:mainfrom
thfroitzheim:ncoord

Conversation

@thfroitzheim
Copy link
Copy Markdown
Contributor

PR to move the coordination number to the mctc-lib (see PR #71 there). This avoids duplicated code in all our projects and makes the addition of new coordination numbers simpler. The full functionality and all tests were moved to mctc-lib. Additionally, I moved the pairwise radii to mctc-lib as common data used in future projects at different places.

This PR starts as a draft pointing to the mctc-lib branch until it is merged.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 17, 2025

Codecov Report

Attention: Patch coverage is 33.33333% with 10 lines in your changes missing coverage. Please review.

Project coverage is 66.75%. Comparing base (fd67bcd) to head (46e0422).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/dftd3/ncoord.f90 33.33% 4 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
- Coverage   67.07%   66.75%   -0.32%     
==========================================
  Files          34       33       -1     
  Lines        4687     4666      -21     
  Branches     1647     1646       -1     
==========================================
- Hits         3144     3115      -29     
- Misses        632      636       +4     
- Partials      911      915       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
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.

Can we reimplement the removed modules based on the new functionality in mctc_ncoord? This would ensure that we don't remove API features in a minor release.

@thfroitzheim
Copy link
Copy Markdown
Contributor Author

Yes, I can do that. I would use these modules then throughout the code. Still, we should remove it with a later 2.0.0 release (I will open an issue after this is done).

@thfroitzheim thfroitzheim marked this pull request as ready for review March 29, 2025 13:23
@thfroitzheim thfroitzheim requested a review from awvwgk March 31, 2025 09:33
Copy link
Copy Markdown
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.

Looks good to me now.

Let's merge this once we have a new mctc-lib release with the full and final API, such that we don't need to depend on the latest git version.

@thfroitzheim
Copy link
Copy Markdown
Contributor Author

We can do that. The only remaining update that has to be reviewed before is in tblite, here.

In the long term, we (@LSeidler and I) also think about pulling the periodic features (lattice vectors and Wigner-Seitz cell) out of the projects and into mctc-lib. But this shouldn't be a problem, since we can keep the API unchanged.

Comment thread config/cmake/Findmctc-lib.cmake Outdated
Comment thread fpm.toml Outdated
Comment thread subprojects/mctc-lib.wrap Outdated
@marvinfriede marvinfriede merged commit 61714e3 into dftd3:main Apr 8, 2025
24 of 26 checks passed
@thfroitzheim thfroitzheim deleted the ncoord branch April 8, 2025 15:18
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.

3 participants