fix(distributed): Fix halo routing for unwrapped periodic migrants - #158
Open
atulcthakur wants to merge 3 commits into
Open
fix(distributed): Fix halo routing for unwrapped periodic migrants#158atulcthakur wants to merge 3 commits into
atulcthakur wants to merge 3 commits into
Conversation
Signed-off-by: atulcthakur <atthakur@nvidia.com>
Collaborator
Author
|
/ok to test 1fb0a60 |
Contributor
Greptile SummaryThe PR canonicalizes periodic fractional coordinates during halo-recipient selection while preserving raw Cartesian positions for transmission.
Important Files Changed
Reviews (3): Last reviewed commit: "test(distributed): allocate NCCL rendezv..." | Re-trigger Greptile |
Signed-off-by: atulcthakur <atthakur@nvidia.com>
Signed-off-by: atulcthakur <atthakur@nvidia.com>
Collaborator
Author
|
/ok to test 467c642 |
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.
ALCHEMI Toolkit Pull Request
Description
During migration hysteresis, an atom can remain owned by its old rank after crossing a periodic boundary.
For example, in a 20 Å cell, an atom at raw
z=25.1is physically atz=5.1. Rank 0 can temporarily retain ownership, but the halo code checks the raw fractional coordinate1.255and fails to send the atom to rank 1. A rank-1 atom atz=5.6then misses a real 0.5 Å interaction and receives the wrong force.Fix
Wrap fractional coordinates into the primary cell on periodic axes before deciding which ranks need the atom as a ghost.
The original Cartesian coordinates are still transmitted. The receiver’s neighbor list applies the required periodic image.
Type of Change
Related Issues
Changes Made
nvalchemi/distributed/_core/particle_halo.pytest/distributed/test_deferred_migration_halo.pyTesting
make pytest)make lint)Checklist
Additional Notes
Tip
This repository uses Greptile, an AI code review service, to help conduct
pull request reviews. We encourage contributors to read and consider suggestions
made by Greptile, but note that human maintainers will provide the necessary
reviews for merging: Greptile's comments are not a qualitative judgement
of your code, nor is it an indication that the PR will be accepted/rejected.
We encourage the use of emoji reactions to Greptile comments, depending on
their usefulness and accuracy.