Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

`unwrap_positions` gives wrong results for triclinic cells

Abierto Apto para principiantes
  • ¿Algo no está bien?
#629 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 2 días

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
85/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
python, pytorch
Área
machine-learning

Línea de trabajo

Abre torch_sim/transforms.py y lee unwrap_positions junto a pbc_wrap_batched, que trata las columnas de cell como vectores de red. Transpone cells una vez al principio de unwrap_positions para que tanto la rama constant-cell como la rama (T, n_systems, 3, 3) usen la misma convención. Ejecuta el reproductor del issue para confirmar que el error máximo baja a ~0, luego extiende el test cubic-cell existente con una triclinic cell para que la regresión quede cubierta.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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.

Lenguaje dominante
Python
Estrellas
502
Forks
108
Merge medio
1 d 1 h
PR fusionados (30 d)
12

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de TorchSim/torch-sim

Todos los issues de TorchSim/torch-sim

Issues similares

Más issues de Python

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.