Run `ty` in lint CI by janosh · Pull Request #339 · TorchSim/torch-sim

janosh · 2025-11-10T02:02:43Z

with number of PRs, contributors and users increasing, it's more important to guard against easy-to-catch type errors. lessens burden on reviewers and allows to merge green PRs with more confidence

thomasloux · 2025-11-10T15:15:47Z

Didn't know this tool, nice to have. But there is a large amount of work to make ty happy with the current code

orionarcher · 2025-11-11T16:15:28Z

I support adding ty but it might be best to wait until the most recent of MACE is on PyPI, last version was released in August. With the most recent version of MACE, it was possible to get the number of type errors down to single digits. @janosh idk if you can bother Ilyas about it.

orionarcher · 2026-02-27T16:31:47Z

MACE finally released v0.3.15 so I'd say this is ready to move forward on

janosh · 2026-02-27T19:31:39Z

@orionarcher set a bunch of agents to work on this and coarsely reviewed their changes. looks good for a start but can definitely be polished. short on time for that. if anyone wants to have a go at that feel free to push to this branch! this will get blocked by merge conflicts quickly

across core, models, neighbors, tests, ...

- Consolidate repeated model kwargs construction in model tests - Simplify example/tutorial guard code while keeping runtime checks - Reuse calculate_momenta and SimState coercion patterns in NVE/NVT/NPT init paths

orionarcher · 2026-02-27T20:48:52Z

This looks broadly good!

I am (CC is) gonna take a swing at this from scratch and see if I can get it to touch fewer LOC. It looks like its changing a decent amount of logic in addition to type signatures and I am curious if that is avoidable.

janosh · 2026-02-27T20:56:58Z

bit more context: started out with 670 type errors. agents bringing that down to 0 even with some hacks like ignoring a few important rules and files is a win. we can always reduce bad config in subsequent PRs as time permits. but would good to have type checking on the rest of code base

if claude can bring the type error count to 0 with fewer changes, i'm all for it!

- consolidate repeated state coercion and system index checks into shared helpers - simplify integrator and optimizer initialization/update paths without behavior changes - de-duplicate noisy test/tutorial patterns while preserving coverage and tolerances fix tests

- fix docs tutorial plotting by detaching tensor data before matplotlib conversion - restore deterministic no-seed rattle behavior used by optimizer regression fixtures - keep FIRE behavior aligned while preserving ty-safe typing adjustments - correct mixed-PBC density scaling path in autobatching and add regression coverage

- fix mixed-PBC memory scaler handling and tighten variable naming in autobatching - restore public NPT C-rescale float-or-tensor signatures with explicit tensor coercion at step call sites - add focused tests for degree-of-freedom counting, initialize_state invalid inputs, and neighbor-list empty-output behavior - simplify neighbor coverage by folding standard_nl into shared ASE-equivalence/edge-case parametrized tests - align neighbor backend docstring fallback order with actual runtime selection

janosh · 2026-02-28T12:50:29Z

I am (CC is) gonna take a swing at this from scratch and see if I can get it to touch fewer LOC.

@orionarcher how did it go?

meantime, i applied some manual polish to the agent edits, mostly aiming to cut any bloat and AI slop (see e8a4cfa, e16893a, ff3bc7b). that worked in the sense that the diff line count in actual code files went down quite a bit. sadly i kept coming across what i think are bugs. i fixed them and added new tests to prevent future regressions. the tests in particular increased the net line count, despite my attempts to cut bloat but useful enough to warrant the lines imo:

mixed-pbc bug in autobatching density scaling: n_atoms_x_density now uses the vectorized volume path only when all pbc axes are periodic (pbc_all), instead of allowing mixed-pbc cases to slip through and get incorrect scaling.
unless i'm missing something this one actually looks pretty serious! if anybody thinks it's not a bug, let me know. otherwise, my best guess for why this wasn't caught earlier is that nobody has used torch-sim for molecules or open surfaces yet (commit, diff)
count_degrees_of_freedom() now sums per-system DoF reduction before subtracting from scalar total. it looks like the function was assuming it's just dealing with a single system. this helper had zero coverage. constraints are tested broadly, but this function wasn’t directly exercised. i added 3 unit tests in tests/test_constraints.py for: no constraints, FixAtoms only, FixCom only, and mixed constraints across multi-system states, with exact DoF assertions. (diff, tests)
fire init/step now normalize scalar/tensor inputs consistently across systems (commit, diff)
optional-neighbor-backend fallback robustness (alchemiops/vesin import and call guards): i think in some environments you’d get a clean ImportError and fallback. in others, you could hit a later “object not callable” style failure path, depending on exactly which optional packages are present.

- guard AtomConstraint/SystemConstraint.merge against empty or mismatched inputs with clear ValueError instead of torch.cat runtime failures - add regression tests for constraint merge edge cases and empty bin-packing inputs - accept float inputs in NPT Nose-Hoover step with explicit tensor coercion - refactor NPT C-rescale step parameter coercion into a shared helper to reduce duplication - harden autobatching binning for empty iterables and tuple-list key reconstruction

- coerce anisotropic NPT c-rescale step inputs via the shared helper for float/tensor consistency - restore ASE LBFGS test logfile to None to avoid noisy stdout in CI - clarify empty dict coverage in autobatching test as intentional API behavior

orionarcher

I didn't do a thorough line by line review but everything I saw LGTM. Just one comment.

Also, one question, what versions of upstream software will ty look at?

Given all the bug fixes, let's just go with this version.

orionarcher · 2026-02-28T18:16:41Z

pyproject.toml

 io = ["ase>=3.26", "phonopy>=2.37.0", "pymatgen>=2025.6.14"]
-symmetry = ["moyopy>=0.3", "spglib>=2.6"]
+symmetry = ["moyopy>=0.3"]
 mace = ["mace-torch>=0.3.14"]


We should probably pin mace to 0.3.15, I suspect ty will fail with earlier versions. It occurs to me that while we are running the tests with lowest and highest deps, I don't think we are doing the same for linting.

good catch, done in b79b8aa

janosh · 2026-02-28T18:47:55Z

Also, one question, what versions of upstream software will ty look at?

ty uses whatever package versions are installed in the environment it runs in. we set no version pins in .pre-commit-config.yaml, meaning we tell prek to install the latest versions of ty, torch, ase, mace-torch

torch-sim/.pre-commit-config.yaml

Line 46 in d81bbf1

additional_dependencies: [ty, torch, ase, mace-torch]

- add shared state.pbc_to_tensor helper and use it across model backends - align NVT/NPT integrator calls with velocity_verlet_step and simplify dt handling - improve safety in trajectory append node typing and expose ParticleLife beta parameter

janosh · 2026-02-28T18:54:43Z

torch_sim/models/particle_life.py

        )
        self.epsilon = torch.tensor(epsilon, dtype=self.dtype, device=self.device)
-        self.beta = torch.tensor(0.3, dtype=self.dtype, device=self.device)
+        self.beta = torch.tensor(beta, dtype=self.dtype, device=self.device)


was beta hard-coded for a reason? maybe @abhijeetgangan?

- add duecredit citations and scalar-or-tensor coercion across NVT/NPT/NVE integrator steps - centralize PBC normalization via state.pbc_to_tensor and apply it across model adapters - remove neighbors __all__ export list and improve SimState/coerce_prng doc safety

janosh · 2026-02-28T19:03:18Z

@CompRhys just noticed agents had removed the due.dcite decorators you'd added. dcite import was removed to satisfy the type checker. I think i restored them all:

NVT integrators (torch_sim/integrators/nvt.py):

nvt_langevin_step — 10.1098/rspa.2016.0138 (Leimkuhler & Matthews, BAOAB Langevin)
nvt_nose_hoover_step — 10.1080/00268979600100761 (Martyna et al., Nose-Hoover chains)
nvt_vrescale_step — 10.1063/1.2408420 (Bussi et al., velocity rescaling)

NPT integrators (torch_sim/integrators/npt.py):

npt_langevin_step — 10.1063/1.4901303 (NPT Langevin)
npt_nose_hoover_step — 10.1080/00268979600100761 (Martyna et al., Nose-Hoover chains)
npt_crescale_anisotropic_step — 10.1063/5.0020514 + 10.3390/app12031139 (C-Rescale barostat)
npt_crescale_independent_lengths_step — same two DOIs
npt_crescale_average_anisotropic_step — same two DOIs
npt_crescale_isotropic_step — 10.1063/5.0020514

thanks for adding those! 👍

- derive VESIN_AVAILABLE from VesinNeighborList is not None instead of separate bool flag set in try/except - remove __all__ from vesin.py and graphpes.py per project rules - bump prek and ty dev dependency versions

- remove deprecated, invalid-assignment, not-subscriptable, possibly-missing-attribute, unsupported-operator from tests override (0 errors each or fixed inline) - add ty: ignore[deprecated] to pbc_wrap_general test call sites - add system_idx None guards in test_constraints and test_monte_carlo - remove stale ty: ignore comments in hybrid_swap_tutorial - rename ft -> tst import alias in test_transforms

- delete pbc_wrap_general (deprecated in favor of wrap_positions) and its 5 test functions - remove unused typing_extensions.deprecated import - add ty: ignore[invalid-return-type] for pymatgen AseAtomsAdaptor return type mismatch in test_fix_symmetry

Removes pymatgen imports from test_fix_symmetry by using ASE's ase.spacegroup.crystal to build the P-6 structure directly.

- switch ty hook from language: python to language: system so it uses the project venv instead of an isolated env missing most deps - remove additional_dependencies (no longer needed with system hook) - remove 15-file blanket source override for unsupported-operator (fixed 3 errors inline) and unresolved-import (narrowed to only files that actually import optional deps) - fix unsupported-operator in autobatching.py, runners.py, state.py

orionarcher force-pushed the main branch from b95f609 to 8c8983c Compare November 23, 2025 17:36

thomasloux mentioned this pull request Nov 24, 2025

Implement SimState.wrap_positions using pbc_wrap_batched #350

Merged

3 tasks

CompRhys added the keep-open PRs to be ignored by StaleBot label Jan 20, 2026

janosh force-pushed the run-ty-in-lint-ci branch from 244a1d5 to 5f03350 Compare February 27, 2026 19:29

fix all ty errors

6331f79

across core, models, neighbors, tests, ...

janosh force-pushed the run-ty-in-lint-ci branch from 5f03350 to 6331f79 Compare February 27, 2026 19:42

Reduce typing boilerplate in tests and integrators

e8a4cfa

- Consolidate repeated model kwargs construction in model tests - Simplify example/tutorial guard code while keeping runtime checks - Reuse calculate_momenta and SimState coercion patterns in NVE/NVT/NPT init paths

janosh added 3 commits February 28, 2026 11:29

janosh added 2 commits February 28, 2026 14:09

orionarcher approved these changes Feb 28, 2026

View reviewed changes

abhijeetgangan approved these changes Feb 28, 2026

View reviewed changes

janosh commented Feb 28, 2026

View reviewed changes

Clean up vesin availability flag and stale exports

e29c3c0

- derive VESIN_AVAILABLE from VesinNeighborList is not None instead of separate bool flag set in try/except - remove __all__ from vesin.py and graphpes.py per project rules - bump prek and ty dev dependency versions

janosh enabled auto-merge (squash) February 28, 2026 19:12

janosh added fix Bug fix and removed keep-open PRs to be ignored by StaleBot labels Feb 28, 2026

janosh added refactor Refactorings types Type all the things labels Feb 28, 2026

janosh added 3 commits February 28, 2026 19:19

Replace pymatgen P-6 builder with ASE crystal()

68b41f2

Removes pymatgen imports from test_fix_symmetry by using ASE's ase.spacegroup.crystal to build the P-6 structure directly.

janosh force-pushed the run-ty-in-lint-ci branch 2 times, most recently from a0291a3 to df002fc Compare February 28, 2026 19:29

janosh force-pushed the run-ty-in-lint-ci branch from df002fc to adfe576 Compare February 28, 2026 19:31

janosh merged commit ce9ac4c into main Feb 28, 2026
68 checks passed

janosh deleted the run-ty-in-lint-ci branch February 28, 2026 19:42

janosh mentioned this pull request Feb 28, 2026

Setup bug bots #472

Open

Conversation

janosh commented Nov 10, 2025

Uh oh!

thomasloux commented Nov 10, 2025

Uh oh!

orionarcher commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

orionarcher commented Feb 27, 2026

Uh oh!

janosh commented Feb 27, 2026

Uh oh!

orionarcher commented Feb 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

janosh commented Feb 27, 2026

Uh oh!

janosh commented Feb 28, 2026

Uh oh!

orionarcher left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

orionarcher Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

janosh Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

janosh commented Feb 28, 2026

Uh oh!

janosh Feb 28, 2026

Choose a reason for hiding this comment

Uh oh!

janosh commented Feb 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

orionarcher commented Nov 11, 2025 •

edited

Loading

orionarcher commented Feb 27, 2026 •

edited

Loading

orionarcher left a comment •

edited

Loading