Skip to content

Keep batched systems separate in VelocityAutoCorrelation - #628

Merged
CompRhys merged 1 commit into
TorchSim:mainfrom
lance-lii:fix-597-vacf-batched
Oct 29, 2026
Merged

CompRhys merged 1 commit into
TorchSim:mainfrom
lance-lii:fix-597-vacf-batched

Conversation

@lance-lii

Copy link
Copy Markdown
Contributor

Summary

Part of #597.

  • Fix: VelocityAutoCorrelation averaged over all atoms in the state, so systems in a batched SimState were mixed into one VACF. It now averages over the atoms of each system (using system_idx) and vacf has shape (n_systems, window_size) for batched input. Single-system input keeps the old (window_size,) shape.
  • The running average now starts from the first window instead of the zero placeholder, so the batched shape works with use_running_average=True.
  • Added a regression test checking that a batched state gives the same VACF per system as running each system on its own.

This only covers the calculator side. The pattern in the original report (ts.integrate with one trajectory file per system, where the reporter passes each split system to the calculator separately) still mixes systems, and I'd leave that for a follow-up PR so this one stays small.

Tests: pytest tests/test_correlations.py passes (14 tests), and ruff check / ruff format --check pass on the changed files.

AI disclosure: I used an AI coding assistant (Claude Code) to help write the fix and test. I reviewed the changes and ran the tests myself.

Checklist

Before a pull request can be merged, the following items must be checked:

  • Doc strings have been added in the Google docstring format.
  • Run ruff on your code.
  • Tests have been added for any new functionality or bug fixes.

@CompRhys

Copy link
Copy Markdown
Member

thanks!

@CompRhys
CompRhys merged commit 35306ec into TorchSim:main Oct 29, 2026
64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants