Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

`unwrap_positions` gives wrong results for triclinic cells

Aperta Adatta ai principianti
  • Qualcosa non va?
#629 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 2 giorni

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
85/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
python, pytorch
Ambito
machine-learning

Direzione di ricerca

Apri torch_sim/transforms.py e leggi unwrap_positions accanto a pbc_wrap_batched, che tratta le colonne di cell come vettori di reticolo. Trasponi cells una volta all'inizio di unwrap_positions in modo che sia il ramo constant-cell sia il ramo (T, n_systems, 3, 3) usino la stessa convenzione. Esegui il reproducer nell'issue per confermare che l'errore massimo scenda a ~0, poi estendi il test cubic-cell esistente con una triclinic cell in modo che la regressione sia coperta.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

torch_sim.transforms.unwrap_positions seems to assume the cell rows are the lattice vectors. It computes fractional coordinates as positions @ inv(cell) and converts back with dfrac @ cell. But SimState.cell, and pbc_wrap_batched, store the lattice vectors as columns. For orthorhombic cells the two are the same, so the current test, which uses a cubic cell, passes. For a triclinic cell the unwrapped trajectory comes out wrong. Both the constant-cell and the time-dependent (T, n_systems, 3, 3) paths are affected.

Repro (CPU, main @ 35306ec):

import torch
import torch_sim.transforms as tst

dtype = torch.float64
cell = torch.tensor([[6.0, 0.0, 0.0],
                     [1.5, 5.0, 0.0],
                     [1.0, 1.2, 4.5]], dtype=dtype).T.unsqueeze(0)  # columns = a, b, c
system_idx = torch.zeros(2, dtype=torch.long)
start = torch.tensor([[5.5, 4.5, 4.0], [1.0, 1.0, 1.0]], dtype=dtype)
step = torch.tensor([[0.3, 0.25, 0.2], [-0.2, 0.1, -0.15]], dtype=dtype)
true_pos = torch.stack([start + i * step for i in range(20)])
wrapped = torch.stack([tst.pbc_wrap_batched(p, cell, system_idx) for p in true_pos])

out = tst.unwrap_positions(wrapped, cell, system_idx)
print((out - true_pos).abs().max())  # expected ~0
print((tst.unwrap_positions(wrapped, cell.mT, system_idx) - true_pos).abs().max())

Expected: a max error of about 0.

Actual: a max error of 2.5. For example, the last frame of atom 0 should be [11.2, 9.25, 7.8] but comes out as [8.7, 9.55, 10.0]. Passing cell.mT gives about 1e-15, so it's only the convention.

Suggested fix: transpose the cell at the top of unwrap_positions (cells = cells.mT), which covers both branches, and add a triclinic case to the tests. I have a small patch with a regression test and can open a PR if that's welcome.

Disclosure: I used an AI assistant to help find and write up this bug, and I checked the repro myself.

Lingua principale
Python
Stelle
502
Fork
108
Merge medio
1g 1h
PR unite (30g)
12

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di TorchSim/torch-sim

Tutte le issue di TorchSim/torch-sim

Issue simili

Altre issue su Python

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.