Per-system pbc: batching periodic and non-periodic systems together
Maintainers usually reply within 2 days
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 35/100
Research direction
Start by reading the discussion in PR #320 and the referenced batch_naive_neighbor_list API, which accepts per-system pbc. Then identify how torch-sim currently represents periodic boundary conditions and where that value reaches neighbor-list calculations. Done means agreeing on a direction and supporting mixed periodic and non-periodic systems in one batch; the issue does not specify an implementation or tests.
Written by the indexing model from the issue text.
Description
In the review of #320 there was a short discussion about making pbc a per-system attribute so that systems with different pbc can be mixed in one batch (https://github.com/TorchSim/torch-sim/pull/320#issuecomment-3500245561).
It was deferred to a separate issue at the time, but I don't think that issue was ever opened, so raising it here.
I think this would be a useful addition. One concern in that discussion was that mixed pbc may complicate the neighbor list calculations (https://github.com/TorchSim/torch-sim/pull/320#issuecomment-3508563598), but the current neighbor list API already supports per-system pbc (e.g. batch_naive_neighbor_list in nvalchemi-toolkit-ops takes pbc of shape (num_systems, 3): https://github.com/NVIDIA/nvalchemi-toolkit-ops/blob/2b7c3c3adfb1ca84b886eecbf14bc60ff6ba1dc2/nvalchemiops/torch/neighbors/batch_naive.py#L765-L767), and I think other methods support similar APIs, so it can be handled without making the neighbor list logic so complicated.
What do you think? If we agree on a direction I'd be happy to work on a PR.
- Dominant language
- Python
- Stars
- 502
- Forks
- 108
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 12
Getting set up
- No Dockerfile or Docker Compose file
- Has a pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from TorchSim/torch-sim
-
`unwrap_positions` gives wrong results for triclinic cellsPossibly taken @lance-lii claimed this 2 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
TorchSim/torch-sim#629 · 1 comment ·
Maintainers usually reply within 2 days
-
Autobatcher memory probe does not recognize cuequivariance out-of-memory errorsPossibly taken @lil-lon claimed this 2 days ago. Open
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
TorchSim/torch-sim#627 · 5 comments · 1 reaction ·
Maintainers usually reply within 2 days
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
Maintainers usually reply within 2 days
-
Add MLIPOps backendsOpenhelp wanted ml-eng-friendly
Difficulty 5/5 Over a week Newbie friendliness 25/100
TorchSim/torch-sim#606 · 6 comments · 1 reaction ·
Maintainers usually reply within 2 days
-
Difficulty 4/5 3-5 days Newbie friendliness 42/100
TorchSim/torch-sim#597 · 7 comments ·
Maintainers usually reply within 2 days
All issues in TorchSim/torch-sim
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
huggingface/smolagents#3106 ·
Maintainers usually reply within 2 days
-
area:scripts area:tests bug priority:later
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Ikalus1988/MisakaNet#4358 ·
Maintainers usually reply within 1 day
-
Discovered ports from service discovery are never propagated to modules in Module.load()Possibly taken @vishalmore90 claimed this today. Open
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
OWASP/Nettacker#1828 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100
aws-samples/sample-per-model-bedrock#708 ·
Maintainers usually reply within 1 day
-
maintenance
Difficulty 2/5 1-3 hours Newbie friendliness 62/100