Repository navigation
Support 1D/2D periodic boundary conditions in cutoff and Wigner-Seitz routines - #155
Merged
Merged
Conversation
…y fallback on failure
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #155 +/- ##
=========================================
- Coverage 11.84% 9.70% -2.14%
=========================================
Files 5 4 -1
Lines 211 206 -5
Branches 97 97
=========================================
- Hits 25 20 -5
Misses 176 176
Partials 10 10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lmseidler
requested changes
Nov 4, 2026
Member
There was a problem hiding this comment.
and regarding all the legacy methods: do we really want to keep them? i think for us it's not common practice, especially since these are not really exposed to a public api in this case.
Member
Author
Only temporarily. The |
Replace the per-dimension LLL/Babai/Fincke-Pohst repetition bounds by one conservative bound in the periodic subspace. For the inverse metric G^-1 of the periodic lattice vectors and the fractional coordinates s of a pair, all competing images satisfy |n_k - s_k| <= sqrt((d0^2 + tol) G^-1_kk), where d0 is the parallel distance to an admissible (non-self) candidate image. - Add get_inverse_metric to mctc_cutoff and use it for cutoff repetitions, wrapping, and the Wigner-Seitz bound, handling 1D/2D/3D alike. A one-element periodicity mask again applies to all three directions. - Drop structure validation from the Wigner-Seitz construction, keep the degenerate-lattice check and a single candidate-count limit. - Keep only integer-range checks for lattice point generation. - Generate the CSR Wigner-Seitz translations with the dense generator and size the image buffers by the number of translations instead of 27. - Add dense versus CSR image parity tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
The conservative translation bound grows quickly for strongly skewed bases and rejected valid sheared cells with too many candidate translations. Apply a pairwise size reduction to the periodic lattice vectors and generate the candidate translations in the reduced basis. Only physical translation vectors are used, so no basis transformation is tracked. Compare cross-basis distances in the exhaustive reference test with a round-off tolerance and add a strongly sheared 3D regression test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
Rounding of the fractional coordinates of distant unwrapped atoms could place an image on the edge of the switching interval just outside the translation box, while the image search on a larger box accepts it. Widen the bound by a small relative tolerance before the integer conversion. Add finite difference tests of the image weight derivatives with respect to the pair vector and to strain for competing images with unequal weights, matching images by their physical translation, and a regression test for images on the edge of the switching interval. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
The linked-cell grid inverted the full lattice matrix and failed for partially periodic structures with zero nonperiodic lattice vectors. Track the periodicity of each grid direction, replace nonperiodic lattice vectors by orthogonal directions spanning the structure perpendicular to the periodic subspace, and only wrap periodic directions in the stencil. Fully periodic and molecular grids are unchanged. Add dense versus CSR Wigner-Seitz parity and CSR versus Verlet tests for a slab and a wire with zero and arbitrary nonperiodic lattice vectors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
The multicharge Ewald and EEQ implementations use get_periodic_axes from mctc_cutoff. Keep it with its explicit loop and use it for the inverse metric and the linked-cell grid. A one-element periodicity mask applies to all three directions, consistent with the lattice point generation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
Add wrapped slab and wire structures with three linked cells along every periodic direction to the CSR versus Verlet tests, which use the direct translation lookup of the linked cells instead of the general stencil. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
Check the cutoff before returning the origin for nonperiodic structures, such that a negative or NaN cutoff fails the same way for every periodicity mask. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
The generic get_pairs and its two wrappers only adapted calling conventions around find_wsc_pairs. Callers now use find_wsc_pairs directly, which returns the total number of competing images, stores the nearest min(nimg, size(list)) of them, and only counts them for a zero-length list. This removes get_pairs from the public API. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
Keep one representative case per behavior and remove overlapping variants: - Wigner-Seitz: drop the separate robustness and 65 degree tests, reduce the exhaustive reference to an oblique slab and a triclinic cell, and the limits to one degenerate basis and one oversized search. Image storage beyond 27 entries uses a small cell with 56 images instead of 738. - find_wsc_pairs: one test for counting and storing tied images. - Cutoff: one failure each for a negative cutoff, the integer range, and a degenerate basis; partial periodicity checks one wire and one slab instead of every mask. - CSR: drop the pyrazole parity case, which the dense test covers against the exhaustive reference, and reduce slab and wire variants to one general stencil and one direct translation lookup each. The slab and wire geometries now contain a pair of neighbors across a periodic boundary after wrapping. Before, the direct translation lookup was reached but never had to resolve a periodic shift. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: marvinfriede <51965259+marvinfriede@users.noreply.github.com>
lmseidler
reviewed
Nov 5, 2026
Member
There was a problem hiding this comment.
overall this looks much better. there are only some minor things to check
marvinfriede
force-pushed
the
feat/periodic-1d-2d
branch
from
November 5, 2026 14:02
539c71a to
68dfec7
Compare
Contributor
|
Marvin, can you also, probably add the "problematic" periodic examples to the mstore? |
Member
Author
|
Contributor
|
I've made a small test of this mctc-lib version for the EEQBC charges, and it worked without errors for both dense and sparse implementations. |
lmseidler
approved these changes
Nov 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend real-space cutoff lattice generation, central-cell coordinate wrapping, and Wigner–Seitz cell construction to arbitrary 1D and 2D periodic boundary conditions alongside 0D and 3D systems, with explicit
error_typeerror handling.I made the error argument required for most functions. However, since some are in the public API, I added legacy overloads that retain the old behavior (no or optional error propagation). In the future, we can just delete these.