-
Notifications
You must be signed in to change notification settings - Fork 39
Rename batch to system #217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change systematically replaces all references to "batch" and "batch indices" with "system" and "system indices" throughout the codebase, including core library modules, models, optimizers, integrators, transforms, IO, and all tests and tutorials. All relevant variable names, attributes, docstrings, comments, and function signatures are updated for consistency. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SimState
participant Model
participant Optimizer
participant Integrator
User->>SimState: Initialize with system_idx tensor
SimState->>Model: Pass system_idx for per-system computation
Model->>SimState: Compute outputs (energy, stress) per system
SimState->>Optimizer: Optimize positions/cells per system
SimState->>Integrator: Integrate dynamics per system
Integrator->>SimState: Update state using system_idx
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
torch_sim/optimizers.py (1)
1144-1150
: Same per-system mass aggregation issue as aboveThe Frechet‐cell initialiser repeats the
segment_reduce
pattern with the same
assumptions and should be fixed identically. Please apply the scatter-based
solution here as well.
🧹 Nitpick comments (8)
examples/scripts/2_Structural_optimization/2.5_MACE_UnitCellFilter_Gradient_Descent.py (1)
86-89
: Minor wording tweak left behindComment now says “atoms per system”, but the surrounding script still labels itself “Batched …” (top-of-file docstring) and uses variables such as
positions_lr
“for all batches”. For perfect consistency with the new terminology, consider updating those occurrences in a follow-up sweep.examples/scripts/2_Structural_optimization/2.6_MACE_UnitCellFilter_FIRE.py (1)
82-86
: Terminology consistencySame note as in the gradient-descent script: the inline comment was updated, but other text (“Batched …”, etc.) still refers to batch. Recommend aligning those in a later tidy-up commit.
examples/scripts/2_Structural_optimization/2.7_MACE_FrechetCellFilter_FIRE.py (1)
81-85
: Left-over “batch” wording elsewhereComment is fixed here, but the file header and logging strings still talk about batches. Harmonising wording across the examples will avoid reader confusion.
torch_sim/math.py (1)
995-1005
: Docstring still mixes “batch” and “system” termsLine 1003 was corrected to
n_systems
, but the earlier parameter docs still reference batch membership and the argument itself isbatch_indices
. If renaming the parameter is out of scope, at least update the wording to avoid contradicting the return description:- batch_indices: Tensor of shape [N_total_entities] indicating batch membership. + batch_indices: Tensor of shape [N_total_entities] indicating system membership + (kept as *batch_indices* for backward compatibility).Tiny polish, but it keeps the documentation coherent.
tests/models/conftest.py (2)
172-172
: Consider renaming variable for consistency.The attribute reference has been correctly updated to
sim_state.system_idx
. However, consider renaming the variable fromog_batch
toog_system_idx
for better clarity and consistency with the new terminology.- og_batch = sim_state.system_idx.clone() + og_system_idx = sim_state.system_idx.clone()
180-180
: Update assertion to use consistent variable name.The assertion has been correctly updated to use
sim_state.system_idx
. If the variable is renamed as suggested above, this assertion should also be updated accordingly.- assert torch.allclose(og_batch, sim_state.system_idx) + assert torch.allclose(og_system_idx, sim_state.system_idx)torch_sim/integrators/npt.py (1)
1215-1225
: Consider using the existing calc_kinetic_energy function with system_idx.The manual loop for computing kinetic energy per system could be replaced with the existing
calc_kinetic_energy
function that supportssystem_idx
parameter:- # Compute kinetic energy contribution per system - # Split momenta and masses by system - KE_per_system = torch.zeros( - n_systems, device=positions.device, dtype=positions.dtype - ) - for b in range(n_systems): - system_mask = system_idx == b - if system_mask.any(): - system_momenta = momenta[system_mask] - system_masses = masses[system_mask] - KE_per_system[b] = calc_kinetic_energy(system_momenta, system_masses) + # Compute kinetic energy contribution per system + KE_per_system = calc_kinetic_energy(momenta, masses, system_idx=system_idx)This would be more efficient and consistent with the codebase patterns.
torch_sim/optimizers.py (1)
119-135
: Guard against device / dtype mismatch when a tensor learning-rate is suppliedIf the caller passes
lr
as an existing tensor on a different device or with a
different dtype than the model, the slice operation
lr[state.system_idx]
will raise an error.
Convert external tensors to the optimiser’s(device,dtype)
first.- # Get per-atom learning rates by mapping batch learning rates to atoms + # Get per-atom learning rates; ensure `lr` lives on the correct device/dtype if isinstance(lr, float): lr = torch.full((state.n_systems,), lr, device=device, dtype=dtype) + else: + lr = lr.to(device=device, dtype=dtype) atom_lr = lr[state.system_idx].unsqueeze(-1) # shape: (total_atoms, 1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (67)
.gitignore
(1 hunks)examples/scripts/1_Introduction/1.2_MACE.py
(1 hunks)examples/scripts/2_Structural_optimization/2.3_MACE_Gradient_Descent.py
(1 hunks)examples/scripts/2_Structural_optimization/2.5_MACE_UnitCellFilter_Gradient_Descent.py
(1 hunks)examples/scripts/2_Structural_optimization/2.6_MACE_UnitCellFilter_FIRE.py
(1 hunks)examples/scripts/2_Structural_optimization/2.7_MACE_FrechetCellFilter_FIRE.py
(1 hunks)examples/scripts/3_Dynamics/3.10_Hybrid_swap_mc.py
(1 hunks)examples/scripts/3_Dynamics/3.11_Lennard_Jones_NPT_Langevin.py
(2 hunks)examples/scripts/3_Dynamics/3.12_MACE_NPT_Langevin.py
(5 hunks)examples/scripts/3_Dynamics/3.13_MACE_NVE_non_pbc.py
(1 hunks)examples/scripts/3_Dynamics/3.2_MACE_NVE.py
(1 hunks)examples/scripts/3_Dynamics/3.3_MACE_NVE_cueq.py
(1 hunks)examples/scripts/3_Dynamics/3.4_MACE_NVT_Langevin.py
(1 hunks)examples/scripts/3_Dynamics/3.5_MACE_NVT_Nose_Hoover.py
(1 hunks)examples/scripts/3_Dynamics/3.6_MACE_NVT_Nose_Hoover_temp_profile.py
(1 hunks)examples/scripts/3_Dynamics/3.7_Lennard_Jones_NPT_Nose_Hoover.py
(2 hunks)examples/scripts/3_Dynamics/3.8_MACE_NPT_Nose_Hoover.py
(4 hunks)examples/scripts/3_Dynamics/3.9_MACE_NVT_staggered_stress.py
(1 hunks)examples/scripts/4_High_level_api/4.2_auto_batching_api.py
(1 hunks)examples/scripts/5_Workflow/5.2_In_Flight_WBM.py
(1 hunks)examples/scripts/7_Others/7.3_Batched_neighbor_list.py
(2 hunks)examples/scripts/7_Others/7.6_Compare_ASE_to_VV_FIRE.py
(10 hunks)examples/tutorials/autobatching_tutorial.py
(2 hunks)examples/tutorials/high_level_tutorial.py
(1 hunks)examples/tutorials/hybrid_swap_tutorial.py
(1 hunks)examples/tutorials/low_level_tutorial.py
(1 hunks)examples/tutorials/state_tutorial.py
(6 hunks)tests/models/conftest.py
(1 hunks)tests/test_autobatching.py
(6 hunks)tests/test_correlations.py
(1 hunks)tests/test_integrators.py
(6 hunks)tests/test_io.py
(6 hunks)tests/test_monte_carlo.py
(5 hunks)tests/test_neighbors.py
(3 hunks)tests/test_optimizers.py
(9 hunks)tests/test_runners.py
(9 hunks)tests/test_state.py
(18 hunks)tests/test_trajectory.py
(5 hunks)tests/test_transforms.py
(10 hunks)torch_sim/autobatching.py
(12 hunks)torch_sim/integrators/md.py
(7 hunks)torch_sim/integrators/npt.py
(51 hunks)torch_sim/integrators/nve.py
(5 hunks)torch_sim/integrators/nvt.py
(10 hunks)torch_sim/io.py
(12 hunks)torch_sim/math.py
(1 hunks)torch_sim/models/fairchem.py
(1 hunks)torch_sim/models/graphpes.py
(1 hunks)torch_sim/models/interface.py
(4 hunks)torch_sim/models/lennard_jones.py
(2 hunks)torch_sim/models/mace.py
(9 hunks)torch_sim/models/metatomic.py
(2 hunks)torch_sim/models/morse.py
(2 hunks)torch_sim/models/orb.py
(3 hunks)torch_sim/models/particle_life.py
(2 hunks)torch_sim/models/sevennet.py
(2 hunks)torch_sim/models/soft_sphere.py
(4 hunks)torch_sim/monte_carlo.py
(5 hunks)torch_sim/neighbors.py
(3 hunks)torch_sim/optimizers.py
(38 hunks)torch_sim/quantities.py
(5 hunks)torch_sim/runners.py
(8 hunks)torch_sim/state.py
(23 hunks)torch_sim/trajectory.py
(6 hunks)torch_sim/transforms.py
(15 hunks)torch_sim/typing.py
(1 hunks)torch_sim/workflows/a2c.py
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (31)
examples/scripts/3_Dynamics/3.11_Lennard_Jones_NPT_Langevin.py (1)
torch_sim/quantities.py (3)
calc_kT
(23-69)get_pressure
(133-141)calc_kinetic_energy
(96-130)
examples/scripts/3_Dynamics/3.7_Lennard_Jones_NPT_Nose_Hoover.py (2)
torch_sim/quantities.py (2)
calc_kT
(23-69)calc_kinetic_energy
(96-130)torch_sim/integrators/npt.py (2)
momenta
(70-72)npt_nose_hoover_invariant
(1563-1672)
examples/scripts/3_Dynamics/3.5_MACE_NVT_Nose_Hoover.py (2)
torch_sim/quantities.py (1)
calc_kT
(23-69)torch_sim/integrators/nvt.py (1)
nvt_nose_hoover_invariant
(448-517)
examples/scripts/2_Structural_optimization/2.3_MACE_Gradient_Descent.py (1)
torch_sim/io.py (1)
atoms_to_state
(180-245)
tests/test_monte_carlo.py (1)
torch_sim/monte_carlo.py (1)
validate_permutation
(127-141)
torch_sim/workflows/a2c.py (2)
torch_sim/state.py (3)
n_systems
(202-204)device
(142-144)dtype
(147-149)torch_sim/models/interface.py (4)
device
(107-109)device
(112-116)dtype
(119-121)dtype
(124-128)
examples/scripts/3_Dynamics/3.6_MACE_NVT_Nose_Hoover_temp_profile.py (1)
torch_sim/quantities.py (1)
calc_kT
(23-69)
examples/tutorials/autobatching_tutorial.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
examples/scripts/5_Workflow/5.2_In_Flight_WBM.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
torch_sim/models/soft_sphere.py (1)
torch_sim/state.py (2)
SimState
(26-353)n_systems
(202-204)
examples/scripts/3_Dynamics/3.10_Hybrid_swap_mc.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
examples/scripts/4_High_level_api/4.2_auto_batching_api.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
examples/scripts/7_Others/7.6_Compare_ASE_to_VV_FIRE.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
torch_sim/models/interface.py (1)
torch_sim/state.py (1)
clone
(239-255)
examples/scripts/3_Dynamics/3.4_MACE_NVT_Langevin.py (1)
torch_sim/quantities.py (1)
calc_kT
(23-69)
examples/tutorials/hybrid_swap_tutorial.py (1)
torch_sim/state.py (3)
n_systems
(202-204)device
(142-144)dtype
(147-149)
examples/scripts/3_Dynamics/3.8_MACE_NPT_Nose_Hoover.py (1)
torch_sim/quantities.py (3)
calc_kT
(23-69)get_pressure
(133-141)calc_kinetic_energy
(96-130)
torch_sim/models/orb.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
torch_sim/runners.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
torch_sim/models/graphpes.py (1)
torch_sim/state.py (1)
n_systems
(202-204)
tests/test_correlations.py (1)
torch_sim/state.py (2)
n_systems
(202-204)split
(281-290)
torch_sim/quantities.py (2)
torch_sim/state.py (4)
momenta
(360-366)n_systems
(202-204)device
(142-144)dtype
(147-149)torch_sim/integrators/md.py (1)
velocities
(44-48)
torch_sim/integrators/nve.py (1)
torch_sim/integrators/md.py (1)
calculate_momenta
(51-109)
tests/test_autobatching.py (3)
torch_sim/state.py (5)
n_systems
(202-204)device
(142-144)dtype
(147-149)batch
(175-181)batch
(184-190)tests/conftest.py (3)
device
(24-25)dtype
(29-30)lj_model
(34-45)torch_sim/autobatching.py (1)
BinningAutoBatcher
(417-699)
torch_sim/autobatching.py (1)
torch_sim/state.py (3)
n_atoms
(152-154)n_systems
(202-204)concatenate_states
(809-895)
tests/test_state.py (2)
torch_sim/state.py (12)
_normalize_system_indices
(400-437)SimState
(26-353)_slice_state
(762-806)n_atoms
(152-154)dtype
(147-149)device
(142-144)n_atoms_per_system
(157-163)n_systems
(202-204)batch
(175-181)batch
(184-190)n_batches
(193-199)n_atoms_per_batch
(166-172)tests/conftest.py (7)
si_double_sim_state
(323-325)si_sim_state
(142-144)si_atoms
(89-91)fe_atoms
(83-85)fe_supercell_sim_state
(298-302)dtype
(29-30)device
(24-25)
torch_sim/integrators/npt.py (3)
torch_sim/state.py (7)
device
(142-144)dtype
(147-149)n_systems
(202-204)n_atoms_per_system
(157-163)momenta
(360-366)to
(321-335)clone
(239-255)torch_sim/integrators/md.py (2)
velocities
(44-48)calculate_momenta
(51-109)torch_sim/quantities.py (1)
calc_kinetic_energy
(96-130)
torch_sim/trajectory.py (2)
torch_sim/state.py (2)
n_systems
(202-204)split
(281-290)tests/test_correlations.py (1)
split
(39-42)
torch_sim/monte_carlo.py (1)
torch_sim/state.py (3)
device
(142-144)n_systems
(202-204)n_atoms
(152-154)
torch_sim/integrators/nvt.py (4)
torch_sim/integrators/md.py (1)
calculate_momenta
(51-109)torch_sim/quantities.py (1)
calc_kinetic_energy
(96-130)torch_sim/integrators/npt.py (1)
momenta
(70-72)torch_sim/state.py (2)
momenta
(360-366)n_atoms_per_system
(157-163)
torch_sim/optimizers.py (2)
torch_sim/state.py (10)
n_systems
(202-204)device
(142-144)dtype
(147-149)to
(321-335)deform_grad
(391-397)clone
(239-255)DeformGradMixin
(356-397)_deform_grad
(379-389)row_vector_cell
(226-228)row_vector_cell
(231-237)torch_sim/math.py (1)
batched_vdot
(992-1022)
🪛 GitHub Check: codecov/patch
torch_sim/autobatching.py
[warning] 296-297: torch_sim/autobatching.py#L296-L297
Added lines #L296 - L297 were not covered by tests
[warning] 346-346: torch_sim/autobatching.py#L346
Added line #L346 was not covered by tests
[warning] 936-936: torch_sim/autobatching.py#L936
Added line #L936 was not covered by tests
[warning] 942-942: torch_sim/autobatching.py#L942
Added line #L942 was not covered by tests
[warning] 1022-1023: torch_sim/autobatching.py#L1022-L1023
Added lines #L1022 - L1023 were not covered by tests
[warning] 1028-1029: torch_sim/autobatching.py#L1028-L1029
Added lines #L1028 - L1029 were not covered by tests
[warning] 1060-1060: torch_sim/autobatching.py#L1060
Added line #L1060 was not covered by tests
torch_sim/integrators/md.py
[warning] 83-83: torch_sim/integrators/md.py#L83
Added line #L83 was not covered by tests
[warning] 90-90: torch_sim/integrators/md.py#L90
Added line #L90 was not covered by tests
[warning] 95-96: torch_sim/integrators/md.py#L95-L96
Added lines #L95 - L96 were not covered by tests
torch_sim/integrators/npt.py
[warning] 109-109: torch_sim/integrators/npt.py#L109
Added line #L109 was not covered by tests
[warning] 112-112: torch_sim/integrators/npt.py#L112
Added line #L112 was not covered by tests
[warning] 120-120: torch_sim/integrators/npt.py#L120
Added line #L120 was not covered by tests
[warning] 132-132: torch_sim/integrators/npt.py#L132
Added line #L132 was not covered by tests
[warning] 139-139: torch_sim/integrators/npt.py#L139
Added line #L139 was not covered by tests
[warning] 246-246: torch_sim/integrators/npt.py#L246
Added line #L246 was not covered by tests
[warning] 249-249: torch_sim/integrators/npt.py#L249
Added line #L249 was not covered by tests
[warning] 286-286: torch_sim/integrators/npt.py#L286
Added line #L286 was not covered by tests
[warning] 288-288: torch_sim/integrators/npt.py#L288
Added line #L288 was not covered by tests
[warning] 291-292: torch_sim/integrators/npt.py#L291-L292
Added lines #L291 - L292 were not covered by tests
[warning] 294-294: torch_sim/integrators/npt.py#L294
Added line #L294 was not covered by tests
[warning] 355-355: torch_sim/integrators/npt.py#L355
Added line #L355 was not covered by tests
[warning] 359-359: torch_sim/integrators/npt.py#L359
Added line #L359 was not covered by tests
[warning] 361-361: torch_sim/integrators/npt.py#L361
Added line #L361 was not covered by tests
[warning] 420-420: torch_sim/integrators/npt.py#L420
Added line #L420 was not covered by tests
[warning] 422-422: torch_sim/integrators/npt.py#L422
Added line #L422 was not covered by tests
[warning] 424-424: torch_sim/integrators/npt.py#L424
Added line #L424 was not covered by tests
[warning] 427-428: torch_sim/integrators/npt.py#L427-L428
Added lines #L427 - L428 were not covered by tests
[warning] 498-498: torch_sim/integrators/npt.py#L498
Added line #L498 was not covered by tests
[warning] 505-506: torch_sim/integrators/npt.py#L505-L506
Added lines #L505 - L506 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (36)
- GitHub Check: test-examples (examples/scripts/5_Workflow/5.3_Elastic.py)
- GitHub Check: test-examples (examples/scripts/2_Structural_optimization/2.7_MACE_FrechetCellFilter_FIRE.py)
- GitHub Check: test-examples (examples/scripts/5_Workflow/5.1_a2c_silicon_batched.py)
- GitHub Check: test-examples (examples/scripts/2_Structural_optimization/2.1_Lennard_Jones_FIRE.py)
- GitHub Check: test-examples (examples/scripts/2_Structural_optimization/2.4_MACE_FIRE.py)
- GitHub Check: test-examples (examples/scripts/2_Structural_optimization/2.5_MACE_UnitCellFilter_Gradient_Descen...
- GitHub Check: test-examples (examples/scripts/2_Structural_optimization/2.3_MACE_Gradient_Descent.py)
- GitHub Check: test-examples (examples/scripts/4_High_level_api/4.1_high_level_api.py)
- GitHub Check: test-examples (examples/scripts/2_Structural_optimization/2.6_MACE_UnitCellFilter_FIRE.py)
- GitHub Check: test-examples (examples/scripts/3_Dynamics/3.6_MACE_NVT_Nose_Hoover_temp_profile.py)
- GitHub Check: test-examples (examples/scripts/3_Dynamics/3.13_MACE_NVE_non_pbc.py)
- GitHub Check: test-examples (examples/scripts/3_Dynamics/3.12_MACE_NPT_Langevin.py)
- GitHub Check: test-examples (examples/scripts/3_Dynamics/3.8_MACE_NPT_Nose_Hoover.py)
- GitHub Check: test-examples (examples/scripts/7_Others/7.6_Compare_ASE_to_VV_FIRE.py)
- GitHub Check: test-examples (examples/scripts/7_Others/7.4_Velocity_AutoCorrelation.py)
- GitHub Check: test-examples (examples/scripts/7_Others/7.2_Stress_autograd.py)
- GitHub Check: test-examples (examples/tutorials/autobatching_tutorial.py)
- GitHub Check: test-examples (examples/tutorials/high_level_tutorial.py)
- GitHub Check: test-examples (examples/tutorials/low_level_tutorial.py)
- GitHub Check: test-model (ubuntu-latest, 3.12, lowest-direct, sevenn, tests/models/test_sevennet.py)
- GitHub Check: test-model (macos-14, 3.12, lowest-direct, metatomic, tests/models/test_metatomic.py)
- GitHub Check: test-model (macos-14, 3.12, lowest-direct, mace, tests/test_elastic.py)
- GitHub Check: test-model (macos-14, 3.12, lowest-direct, mace, tests/test_optimizers_vs_ase.py)
- GitHub Check: test-model (ubuntu-latest, 3.12, lowest-direct, orb, tests/models/test_orb.py)
- GitHub Check: test-model (macos-14, 3.11, highest, mace, tests/models/test_mace.py)
- GitHub Check: test-model (ubuntu-latest, 3.12, lowest-direct, mattersim, tests/models/test_mattersim.py)
- GitHub Check: test-model (ubuntu-latest, 3.12, lowest-direct, mace, tests/test_elastic.py)
- GitHub Check: test-model (ubuntu-latest, 3.12, lowest-direct, mace, tests/test_optimizers_vs_ase.py)
- GitHub Check: test-model (ubuntu-latest, 3.11, highest, sevenn, tests/models/test_sevennet.py)
- GitHub Check: test-model (ubuntu-latest, 3.12, lowest-direct, fairchem, tests/models/test_fairchem.py)
- GitHub Check: test-model (ubuntu-latest, 3.11, highest, mace, tests/test_elastic.py)
- GitHub Check: test-model (ubuntu-latest, 3.11, highest, orb, tests/models/test_orb.py)
- GitHub Check: test-model (ubuntu-latest, 3.11, highest, fairchem, tests/models/test_fairchem.py)
- GitHub Check: test-core (ubuntu-latest, 3.12, lowest-direct)
- GitHub Check: test-core (macos-14, 3.12, lowest-direct)
- GitHub Check: build-docs
🔇 Additional comments (207)
.gitignore (1)
37-39
: 👍 Ignoring the.vscode/
workspace folder is sensibleKeeps editor-specific clutter out of the repo.
No further action required.examples/scripts/5_Workflow/5.2_In_Flight_WBM.py (1)
86-86
: LGTM! Consistent with systematic renaming effort.The change from
state.n_batches
tostate.n_systems
correctly aligns with the codebase-wide renaming from "batch" to "system" terminology. The print statement now accurately reflects that it's reporting the number of systems rather than batches.examples/scripts/3_Dynamics/3.10_Hybrid_swap_mc.py (1)
89-89
: LGTM! Correct tensor dimensionality with updated terminology.The change from
md_state.n_batches
tomd_state.n_systems
correctly updates the tensor initialization to use the new system-based terminology while maintaining the proper dimensionality for tracking permutations across systems.examples/scripts/3_Dynamics/3.13_MACE_NVE_non_pbc.py (1)
80-80
: LGTM! Parameter name updated to match new function signature.The change from
batch=state.batch
tosystem_idx=state.system_idx
correctly updates the function call to use the new parameter name and attribute, ensuring proper system indexing for kinetic energy calculations.examples/scripts/4_High_level_api/4.2_auto_batching_api.py (1)
79-79
: LGTM! Print statement updated for consistency.The change from
state.n_batches
tostate.n_systems
correctly updates the debug print statement to use the new system-based terminology, maintaining consistency with the codebase-wide renaming effort.examples/scripts/3_Dynamics/3.2_MACE_NVE.py (1)
91-91
: LGTM! Function call updated with correct parameter and attribute names.The change from
batch=state.batch
tosystem_idx=state.system_idx
correctly updates both the parameter name and the attribute reference to align with the systematic renaming from "batch" to "system" terminology throughout the codebase.examples/scripts/3_Dynamics/3.9_MACE_NVT_staggered_stress.py (1)
68-68
: LGTM: Parameter name updated consistently with function signature.The change from
batch=state.batch
tosystem_idx=state.system_idx
correctly aligns with the updatedcalc_kT
function signature intorch_sim/quantities.py
(lines 23-69), which now acceptssystem_idx
parameter for system-specific temperature calculations.examples/scripts/3_Dynamics/3.3_MACE_NVE_cueq.py (1)
74-74
: LGTM: Parameter name updated consistently with function signature.The change from
batch=state.batch
tosystem_idx=state.system_idx
correctly aligns with the updatedcalc_kinetic_energy
function signature intorch_sim/quantities.py
(lines 96-130), which now acceptssystem_idx
parameter for system-specific kinetic energy calculations.examples/scripts/3_Dynamics/3.7_Lennard_Jones_NPT_Nose_Hoover.py (1)
126-128
: LGTM: All parameter names updated consistently with function signatures.The systematic replacement of
batch=state.batch
withsystem_idx=state.system_idx
in bothcalc_kT
andcalc_kinetic_energy
function calls is correct and consistent with the updated function signatures intorch_sim/quantities.py
. This maintains the same functionality while using the updated system-based terminology.Also applies to: 135-135, 150-150, 157-159
examples/tutorials/high_level_tutorial.py (1)
378-378
: LGTM: Documentation updated to reflect new terminology.The change from
n_batches
ton_systems
in the documentation correctly reflects the updated terminology used throughout the codebase. This improves consistency and clarity for users.examples/scripts/3_Dynamics/3.4_MACE_NVT_Langevin.py (1)
86-88
: LGTM: Parameter names updated consistently with function signature.The changes from
batch=state.batch
tosystem_idx=state.system_idx
in bothcalc_kT
function calls are correct and consistent with the updated function signature intorch_sim/quantities.py
(lines 23-69). This maintains the same temperature calculation functionality while using the updated system-based terminology.Also applies to: 95-95
examples/scripts/3_Dynamics/3.5_MACE_NVT_Nose_Hoover.py (2)
71-73
: LGTM! Function call updated correctly.The
calc_kT
function call has been correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, which aligns with the systematic renaming throughout the codebase.
81-81
: LGTM! Function call updated correctly.The
calc_kT
function call has been correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, which aligns with the systematic renaming throughout the codebase.torch_sim/typing.py (1)
18-18
: LGTM! Type definition updated correctly.The
StateKey
type literal has been correctly updated to include"system_idx"
instead of"batch"
, which maintains type safety after the attribute renaming in the state objects.examples/tutorials/hybrid_swap_tutorial.py (1)
136-136
: LGTM! Property reference updated correctly.The tensor initialization has been correctly updated to use
md_state.n_systems
instead ofmd_state.n_batches
, which aligns with the property renaming in the state object.examples/scripts/3_Dynamics/3.12_MACE_NPT_Langevin.py (5)
71-73
: LGTM! Function call updated correctly.The
calc_kT
function call has been correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, which aligns with the systematic renaming throughout the codebase.
88-90
: LGTM! Function call updated correctly.The
calc_kT
function call has been correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, which aligns with the systematic renaming throughout the codebase.
98-102
: LGTM! Function call updated correctly.The
calc_kinetic_energy
function call has been correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, which aligns with the systematic renaming throughout the codebase.
116-116
: LGTM! Function call updated correctly.The
calc_kT
function call has been correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, which aligns with the systematic renaming throughout the codebase.
125-127
: LGTM! Function call updated correctly.The
calc_kinetic_energy
function call has been correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, which aligns with the systematic renaming throughout the codebase.examples/scripts/3_Dynamics/3.11_Lennard_Jones_NPT_Langevin.py (4)
122-124
: LGTM - Consistent parameter renamingThe
calc_kT
function call correctly uses the updatedsystem_idx
parameter instead of the deprecatedbatch
parameter. This aligns with the function signature changes intorch_sim/quantities.py
(lines 23-69).
129-131
: LGTM - Consistent parameter renamingThe
calc_kinetic_energy
function call correctly uses the updatedsystem_idx
parameter instead of the deprecatedbatch
parameter. This aligns with the function signature changes intorch_sim/quantities.py
(lines 96-130).
143-145
: LGTM - Consistent parameter renamingThe final temperature calculation correctly uses the updated
system_idx
parameter, maintaining consistency with the refactoring pattern.
151-153
: LGTM - Consistent parameter renamingThe final kinetic energy calculation correctly uses the updated
system_idx
parameter, completing the consistent refactoring across all function calls in this script.torch_sim/models/metatomic.py (2)
77-77
: LGTM - Improved docstring clarityThe docstring update from "batch indices" to "system indices" improves semantic clarity and aligns with the broader refactoring to use more descriptive terminology.
203-203
: LGTM - Consistent attribute renamingThe logic correctly uses
state.system_idx
instead of the deprecatedstate.batch
attribute. This maintains the same functionality while using the renamed attribute that better represents the concept of system indexing.examples/tutorials/autobatching_tutorial.py (3)
252-252
: LGTM - Consistent property renamingThe variable assignment correctly uses the updated
n_systems
property instead of the deprecatedn_batches
property. This aligns with the property definition intorch_sim/state.py
(lines 202-204).
282-282
: LGTM - Consistent property renamingThe assertion correctly uses the updated
n_systems
property to verify all states have been processed. This maintains the same logic while using the renamed property.
286-286
: LGTM - Consistent property renamingThe standalone expression correctly uses the updated
n_systems
property, completing the consistent refactoring across all property references in this tutorial.examples/scripts/3_Dynamics/3.8_MACE_NPT_Nose_Hoover.py (5)
72-74
: LGTM - Consistent parameter renamingThe
calc_kT
function call correctly uses the updatedsystem_idx
parameter, aligning with the function signature changes intorch_sim/quantities.py
.
91-93
: LGTM - Consistent parameter renamingThe
calc_kT
function call in the NPT loop correctly uses the updatedsystem_idx
parameter, maintaining consistency with the refactoring pattern.
101-103
: LGTM - Consistent parameter renamingThe
calc_kinetic_energy
function call correctly uses the updatedsystem_idx
parameter, aligning with the function signature changes intorch_sim/quantities.py
.
114-114
: LGTM - Consistent parameter renamingThe final temperature calculation correctly uses the updated
system_idx
parameter, completing the consistent refactoring in temperature calculations.
122-124
: LGTM - Consistent parameter renamingThe final kinetic energy calculation correctly uses the updated
system_idx
parameter, completing the consistent refactoring across all function calls in this script.examples/tutorials/low_level_tutorial.py (1)
230-232
: LGTM - Consistent parameter renamingThe
calc_kT
function call correctly uses the updatedsystem_idx
parameter instead of the deprecatedbatch
parameter. This aligns with the function signature changes intorch_sim/quantities.py
and maintains consistency with the refactoring pattern across the codebase.examples/scripts/3_Dynamics/3.6_MACE_NVT_Nose_Hoover_temp_profile.py (1)
166-166
: LGTM: Consistent with system terminology update.The parameter change from
batch=state.batch
tosystem_idx=state.system_idx
correctly aligns with the updatedcalc_kT
function signature shown in the relevant code snippets.examples/scripts/2_Structural_optimization/2.3_MACE_Gradient_Descent.py (2)
96-102
: LGTM: Improved variable naming for clarity.The renaming from
atoms_per_batch
toatoms_per_system
andbatch_indices
tosystem_indices
better reflects the semantic meaning of the data - representing individual atomic systems rather than generic batch processing.
109-109
: LGTM: Print statement updated consistently.The print statement correctly references
state.system_idx
to match the updated attribute name.examples/scripts/1_Introduction/1.2_MACE.py (4)
66-72
: LGTM: Consistent variable renaming improves clarity.The variables
atoms_per_system
andsystem_idx
better convey the semantic meaning of representing individual atomic systems rather than generic batch processing.
78-78
: LGTM: Print statement updated consistently.The print statement correctly uses "System indices" to match the updated variable name.
86-86
: LGTM: Function argument updated to match new parameter name.The argument change from
batch=batch
tosystem_idx=system_idx
correctly aligns with the updated function signature.
91-91
: LGTM: Comments updated to reflect new terminology.The comments correctly reference "systems" instead of "batches" to match the updated variable semantics.
Also applies to: 97-97
tests/test_monte_carlo.py (3)
53-53
: LGTM: Function calls updated to match new API.The
validate_permutation
calls correctly usesystem_idx
parameter, which aligns with the updated function signature shown in the relevant code snippets.Also applies to: 98-100, 110-110
67-69
: LGTM: Assertion logic updated consistently.The test correctly verifies that swaps occur within the same system using the updated
system_idx
attribute.
152-165
: LGTM: Test assertions updated to use new terminology.The test assertions correctly use
system_idx
instead ofbatch
to verify system assignments and atomic number distributions, maintaining the same validation logic with improved naming.torch_sim/models/sevennet.py (2)
184-185
: LGTM: Internal implementation updated consistently.The loop iteration and masking logic correctly use
state.system_idx
instead ofstate.batch
, maintaining identical functionality while improving naming consistency.
248-248
: LGTM: Energy tensor creation updated consistently.The energy tensor creation correctly uses
state.system_idx.max().item() + 1
to maintain the same logic with updated attribute naming.torch_sim/workflows/a2c.py (1)
733-735
: LGTM: Consistent tensor shape update for system-based indexing.The tensor shapes for energy and stress logging have been correctly updated from
state.n_batches
tostate.n_systems
, maintaining the same dimensionality while aligning with the new system-based terminology.torch_sim/models/orb.py (3)
105-105
: LGTM: Correct update to system-based indexing.The change from
state.batch
tostate.system_idx
is consistent with the new system-based indexing scheme and maintains the same functionality.
146-146
: LGTM: Proper system count calculation.Computing the number of systems from
state.system_idx.max().item() + 1
is correct and aligns with the new indexing approach.
160-160
: LGTM: Correct mask creation for system filtering.The boolean mask
state.system_idx == i
properly filters atoms belonging to system i, maintaining the same logic with updated terminology.torch_sim/models/graphpes.py (1)
70-71
: LGTM: Consistent system-based iteration and filtering.The changes from
state.n_batches
tostate.n_systems
andstate.batch == i
tostate.system_idx == i
correctly update the iteration and masking logic while maintaining the same functionality.examples/scripts/7_Others/7.3_Batched_neighbor_list.py (3)
21-21
: LGTM: Proper variable renaming in example script.The variable name change from
batch
tosystem_idx
correctly reflects the new system-based indexing terminology.
25-26
: LGTM: Updated comment reflects new terminology.The comment correctly describes the tensor shape as corresponding to
n_systems
instead of batches, maintaining accuracy with the new terminology.
29-29
: LGTM: Function calls updated for new API.The neighbor list function calls have been correctly updated to pass
system_idx
instead ofbatch
, demonstrating proper usage of the new API.Also applies to: 41-41
torch_sim/neighbors.py (3)
769-769
: LGTM: Function parameter updated for consistency.The parameter name change from
batch
tosystem_idx
aligns with the new system-based indexing terminology across the codebase.
787-787
: LGTM: Docstring updated to reflect new parameter name.The docstring correctly describes the
system_idx
parameter, maintaining documentation accuracy with the API changes.
809-809
: LGTM: Internal usage updated consistently.The internal usage of
system_idx
in thetorch.bincount
call is correct and maintains the same functionality with the new parameter name.torch_sim/runners.py (8)
175-175
: LGTM: Progress bar initialization correctly updated.The change from
state.n_batches
tostate.n_systems
properly reflects the new terminology and maintains the correct progress bar behavior for tracking system processing.
197-197
: LGTM: Progress bar update correctly updated.The change from
state.n_batches
tostate.n_systems
in the progress bar update call is consistent with the initialization change and maintains proper progress tracking.
310-312
: LGTM: Docstring correctly updated to reflect system terminology.The docstring update from "batch" to "system" terminology accurately describes that the convergence status is returned per system rather than per batch, improving code clarity.
346-348
: LGTM: Docstring consistently updated to system terminology.The docstring update maintains consistency with the broader refactoring, correctly describing convergence status per system.
375-375
: LGTM: Function parameter docstring updated consistently.The docstring update for the
convergence_fn
parameter correctly describes it as returning a boolean tensor of lengthn_systems
, maintaining consistency with the terminology change.
437-437
: LGTM: Progress bar initialization updated consistently.The change to
state.n_systems
in the optimize function's progress bar initialization is consistent with the other progress bar updates in the file.
548-548
: LGTM: Progress bar initialization updated consistently.The change to
state.n_systems
in the static function's progress bar initialization maintains consistency with the terminology changes throughout the file.
571-571
: LGTM: Progress bar update updated consistently.The change to
sub_state.n_systems
in the progress bar update maintains consistency with the terminology changes and ensures proper progress tracking.tests/test_trajectory.py (6)
35-37
: LGTM: Test fixture attribute updated consistently.The change from
batch_idx
tosystem_idx
in the test fixture aligns with the broader terminology refactoring and maintains test functionality.
681-683
: LGTM: Variable name updated for clarity.The change from
atoms_per_batch
toatoms_per_system
improves code readability and aligns with the system terminology used throughout the refactoring.
701-714
: LGTM: Loop variable renamed consistently.The change from
batch
tosystem_idx
in the loop variable and related usage maintains test functionality while improving semantic clarity.
730-738
: LGTM: Variable names updated consistently.The changes from
batch
tosystem_idx
andsystem_props
improve code clarity and align with the broader refactoring terminology.
770-776
: LGTM: Variable names updated consistently.The systematic renaming from
batch
tosystem_props
andsystem_idx
maintains test functionality while improving semantic consistency.
783-798
: LGTM: Variable names updated consistently.The changes from
batch
tosystem_idx
andsystem_props
in the test function maintain test logic while improving code clarity through consistent terminology.tests/test_correlations.py (2)
34-37
: LGTM: MockState attributes updated consistently.The changes from
n_batches
ton_systems
andbatch
tosystem_idx
align with the broader terminology refactoring while maintaining the mock class functionality for testing.
40-42
: LGTM: Docstring and comment updated consistently.The updates to the docstring and comment reflect the new system terminology, improving code clarity while maintaining the mock class behavior.
tests/test_io.py (6)
51-55
: LGTM: Test assertion updated consistently.The change from
state.batch
tostate.system_idx
in the assertion maintains test functionality while aligning with the broader terminology refactoring.
69-70
: LGTM: Test assertions updated consistently.The changes from
state.batch
tostate.system_idx
in the assertions maintain test functionality while improving semantic clarity.
84-88
: LGTM: Test assertions updated consistently.The changes from
state.batch
tostate.system_idx
in the assertions align with the broader refactoring and maintain proper test validation.
176-180
: LGTM: Test assertions updated consistently.The changes from
state.batch
tostate.system_idx
in the assertions maintain test functionality while improving semantic consistency.
241-245
: LGTM: Test logic updated consistently.The changes from
sim_state.batch
tosim_state.system_idx
in the round-trip test logic maintain test functionality while aligning with the new terminology.
254-254
: LGTM: Test assertion updated consistently.The change from
sim_state.batch
tosim_state.system_idx
in the assertion maintains test validation while improving semantic clarity.tests/test_optimizers.py (9)
131-131
: LGTM: SimState construction updated consistently.The change from
batch
tosystem_idx
in the SimState construction maintains test functionality while aligning with the broader terminology refactoring.
232-232
: LGTM: Variable name updated consistently.The change from
initial_dt_batch
to reflect system terminology maintains test functionality while improving semantic clarity.
265-271
: LGTM: Variable indexing updated consistently.The changes from
batch
tosystem_idx
in the variable indexing maintain test functionality while aligning with the new attribute naming convention.
320-322
: LGTM: Variable indexing updated consistently.The changes from
batch
tosystem_idx
in the velocity indexing maintain test functionality while improving semantic consistency.
348-348
: LGTM: SimState construction updated consistently.The change from
batch
tosystem_idx
in the SimState construction aligns with the broader refactoring and maintains test functionality.
432-432
: LGTM: Tensor shape parameter updated consistently.The change from
n_batches
ton_systems
in the tensor shape parameter aligns with the broader terminology refactoring and maintains correct tensor dimensions.
455-461
: LGTM: Comment and method call updated consistently.The changes from
n_batches
ton_systems
in the comment and frombatch
tosystem_idx
in the method call maintain test functionality while improving semantic clarity.
528-528
: LGTM: SimState construction updated consistently.The change from
batch
tosystem_idx
in the SimState construction maintains test functionality while aligning with the broader terminology refactoring.
877-879
: LGTM: Comment updated consistently.The change from "batch" to "system" terminology in the comment improves code clarity while maintaining the test's intent.
torch_sim/models/fairchem.py (1)
352-362
: LGTM: Consistent renaming from batch to system_idx.The changes correctly replace
state.batch
withstate.system_idx
throughout the forward method. The logic is preserved and the usage oftorch.bincount(state.system_idx)
and tensor sizing withstate.system_idx.size(0)
is appropriate for the new system-based indexing.torch_sim/integrators/nve.py (5)
40-42
: LGTM: Docstring updates reflect new system terminology.The docstring changes from
[n_batches]
to[n_systems]
correctly reflect the updated terminology for tensor shapes.
75-75
: LGTM: Docstring update for kT parameter.The docstring correctly specifies that
kT
can be scalar or have shape[n_systems]
.
91-91
: LGTM: Function call updated to use system_idx.The call to
calculate_momenta
correctly usesstate.system_idx
instead ofstate.batch
, matching the function signature shown in the relevant code snippets fromtorch_sim/integrators/md.py
.
102-102
: LGTM: MDState initialization updated.The
MDState
initialization correctly usessystem_idx=state.system_idx
instead of the deprecated batch attribute.
119-119
: LGTM: Docstring update for dt parameter.The docstring correctly specifies that
dt
can be scalar or have shape[n_systems]
.torch_sim/models/lennard_jones.py (5)
360-360
: LGTM: Docstring updated to use system terminology.The docstring correctly updates from "batch" to "system" terminology in the method description.
369-373
: LGTM: Return value documentation updated.The docstring correctly reflects that energy and stress tensors have shapes
[n_systems]
and[n_systems, 3, 3]
respectively.
380-380
: LGTM: Error description updated.The docstring correctly updates the error description to use "system" instead of "batch".
388-390
: LGTM: Example documentation updated.The example correctly shows that energy and stress have shapes
[n_systems]
and[n_systems, 3, 3]
respectively.
397-400
: LGTM: Runtime validation and iteration updated.The runtime check correctly uses
state.system_idx
and the error message properly references "System" instead of "batch". The iteration over systems correctly usesstate.n_systems
.tests/test_autobatching.py (7)
118-122
: LGTM: Test assertions updated for new system terminology.The test correctly uses
state[1].n_systems == 1
andstate[1].system_idx == 0
to validate split state properties. The comment appropriately explains that system indices are reset to 0 in split states.
475-482
: LGTM: Convergence function updated to use system-based indexing.The convergence function correctly:
- Uses
system_wise_max_force
instead ofbatch_wise_max_force
- Creates tensor with shape
state.n_systems
instead ofn_batches
- Uses
state.system_idx
for scatter_reduce indexing- Returns system-wise convergence criteria
517-517
: LGTM: Variable name updated for consistency.The variable name
optimal_n_systems
correctly reflects the new terminology while maintaining the same logic.
525-527
: LGTM: System counting updated.The system counting logic correctly uses
n_systems
instead ofn_batches
while preserving the test logic.
538-538
: LGTM: Assertion updated for new variable names.The assertion correctly compares
n_systems == optimal_n_systems
using the updated variable names.
564-564
: LGTM: Convergence tensor creation updated.The convergence tensor creation correctly uses
state.n_systems
instead ofn_batches
for sizing.
577-577
: LGTM: Convergence tensor update consistent.The convergence tensor update correctly uses
state.n_systems
for consistency with the new system-based indexing.torch_sim/models/particle_life.py (4)
226-226
: LGTM: Docstring updated for energy tensor shape.The docstring correctly specifies that energy has shape
[n_systems]
.
229-229
: LGTM: Docstring updated for stress tensor shape.The docstring correctly specifies that stress has shape
[n_systems, 3, 3]
.
242-245
: LGTM: Runtime validation updated with clearer error message.The runtime check correctly uses
state.system_idx
instead ofstate.batch
, and the error message clearly indicates thatsystem_idx
can only be inferred for single-system states.
247-247
: LGTM: System iteration updated.The iteration correctly uses
state.n_systems
instead ofstate.n_batches
to process each system.torch_sim/models/soft_sphere.py (5)
384-385
: LGTM: Improved documentation clarityThe documentation update correctly clarifies that the method handles "each system" rather than generic "batched states," which better reflects the atomistic simulation context.
394-394
: LGTM: Consistent shape documentation updatesThe documentation correctly updates tensor shapes to use
n_systems
instead ofn_batches
, maintaining consistency with the new terminology while accurately describing the expected output dimensions.Also applies to: 397-397, 410-410
402-402
: LGTM: Consistent error handling and attribute accessThe error message and state attribute access are correctly updated to use
system_idx
instead ofbatch
, maintaining consistency with the newSimState
API.Also applies to: 418-421
423-423
: LGTM: Correct loop iteration updateThe loop correctly iterates over
state.n_systems
instead ofstate.n_batches
, maintaining the same functional behavior with the updated terminology.
821-821
: LGTM: Consistent terminology updates in SoftSphereMultiModelAll changes in the
SoftSphereMultiModel
class correctly apply the same terminology updates, ensuring consistency across both model implementations.Also applies to: 824-824, 860-863, 865-865
examples/scripts/7_Others/7.6_Compare_ASE_to_VV_FIRE.py (3)
144-144
: LGTM: Consistent state property updatesAll references to
n_batches
are correctly updated ton_systems
, maintaining the same functionality while using the improved terminology.Also applies to: 171-171, 222-222, 233-233
332-332
: LGTM: Consistent variable renamingVariable names are consistently updated from batch-related to system-related terminology (
batch_idx
→system_idx
,all_batches_for_gd
→all_systems_for_gd
), improving code readability and semantic accuracy.Also applies to: 337-337, 353-355, 364-364, 378-378
396-397
: LGTM: Consistent parameter and comment updatesThe comment and parameter name updates (
concatenated_batch_indices
→concatenated_system_indices
,batch
→system_idx
) maintain consistency with the new terminology while preserving functionality.Also applies to: 416-416
tests/test_integrators.py (2)
112-112
: LGTM: Consistent function call parameter updatesAll calls to
calc_kT
are correctly updated to usesystem_idx=state.system_idx
instead ofbatch=state.batch
, maintaining test functionality with the updated API.Also applies to: 175-175, 216-216, 276-276
375-376
: LGTM: Consistent test function and parameter updatesThe test function name and parameter updates (
test_compute_cell_force_atoms_per_batch
→test_compute_cell_force_atoms_per_system
,batch
→system_idx
) ensure the test remains accurate with the refactored API.Also applies to: 392-392
examples/tutorials/state_tutorial.py (2)
34-34
: LGTM: Consistent documentation and comment updatesComments and documentation are correctly updated to use "system" terminology instead of "batch," improving clarity and alignment with the codebase refactoring.
Also applies to: 78-78, 263-263
61-61
: LGTM: Consistent property and attribute access updatesAll property and attribute access are correctly updated to use the new API (
n_systems
instead ofn_batches
,system_idx
instead ofbatch
), ensuring the tutorial remains functional and educational.Also applies to: 69-69, 112-112, 119-119, 151-151, 157-157, 162-162, 256-256
tests/test_neighbors.py (6)
40-40
: LGTM: Docstring updated to reflect new terminology.The docstring correctly updates the return value description from "batch index" to "system index" which better describes the semantic meaning of this tensor.
52-52
: LGTM: Variable renamed consistently.The variable
system_idx
is more descriptive thanbatch
as it represents the system index for each atom.
56-56
: LGTM: Variable usage updated consistently.The assignment correctly uses the renamed
system_idx
variable.
62-62
: LGTM: Return value updated to use renamed variable.The function return statement correctly uses the renamed
system_idx
variable.
559-559
: LGTM: Variable renamed consistently in test function.The variable naming is consistent with the broader refactoring from "batch" to "system_idx" terminology.
563-563
: LGTM: Function call updated with renamed parameter.The function call correctly passes the renamed
system_idx
parameter instead of the oldbatch
parameter.tests/test_transforms.py (10)
253-254
: LGTM: Variable names updated to use new terminology.The mask variables are correctly renamed to use
system_idx
instead ofbatch
, making the code more semantically clear.
271-273
: LGTM: Function call updated with renamed parameter.The function call correctly uses the new
system_idx
parameter name instead of the oldbatch
parameter.
322-322
: LGTM: Function parameter updated consistently.The function call correctly passes the
system_idx
parameter with the appropriate tensor.
349-349
: LGTM: Variable renamed for consistency.The variable
system_idx
is more descriptive than the previous naming convention.
352-352
: LGTM: Function call updated with renamed parameter.The function call correctly uses the updated parameter name.
372-372
: LGTM: Variable renamed consistently.The variable naming follows the new terminology convention.
377-377
: LGTM: Function parameters updated consistently.All function calls correctly use the new
system_idx
parameter name, maintaining consistency throughout the test suite.Also applies to: 385-385, 393-393
405-405
: LGTM: Variable usage updated throughout the test.The variable references are consistently updated to use
system_idx
instead ofbatch
, maintaining the test logic while using more descriptive naming.Also applies to: 410-410, 421-421, 425-425, 428-428, 432-432
439-439
: LGTM: Comments and variable names updated consistently.The comments and variable names are correctly updated to reflect the new "system" terminology while preserving the test logic.
Also applies to: 450-450, 455-455, 456-456, 463-463
849-849
: LGTM: Test documentation and logic updated consistently.The test names, comments, and assertions are correctly updated to use "system" terminology instead of "batch", making the tests more semantically accurate.
Also applies to: 852-852, 865-865, 874-874, 875-875
torch_sim/models/morse.py (6)
359-359
: LGTM: Documentation updated to reflect new terminology.The docstring correctly updates the energy tensor shape description from
[n_batches]
to[n_systems]
, which is more semantically accurate.
362-362
: LGTM: Documentation updated consistently.The stress tensor shape description is correctly updated from
[n_batches, 3, 3]
to[n_systems, 3, 3]
.
375-375
: LGTM: Example documentation updated.The example comment correctly reflects the new energy tensor shape using
[n_systems]
instead of[n_batches]
.
382-382
: LGTM: Code updated to use new attribute name.The conditional check correctly uses
state.system_idx
instead ofstate.batch
, maintaining the same logic with the updated attribute name.
383-385
: LGTM: Error message updated to reflect new terminology.The error message correctly mentions
system_idx
instead ofbatch
, providing clear guidance to users about the updated API.
387-387
: LGTM: Iteration updated to use new property name.The loop correctly uses
state.n_systems
instead ofstate.n_batches
, maintaining the same functionality with the updated property name.torch_sim/autobatching.py (8)
237-237
: LGTM: Consistent terminology updateThe change from
state.n_batches
tostate.n_systems
aligns with the broader codebase refactoring to use more semantically accurate terminology.
296-297
: LGTM: Variable renaming maintains logicThe renaming from
n_batches
ton_systems
and the corresponding usage inconcatenate_states
is consistent with the terminology update while preserving the original functionality.
346-346
: LGTM: Consistent attribute accessThe change from
state.n_batches
tostate.n_systems
maintains consistency with theSimState
class updates referenced in the relevant code snippets.
408-409
: LGTM: Updated error messagingThe print statement correctly uses the new
state.n_systems
attribute, maintaining consistency with the terminology changes while preserving the debugging information.
431-431
: LGTM: Consistent docstring updatesThe parameter descriptions have been updated from "batch" to "system" terminology, improving semantic clarity while maintaining accurate documentation.
Also applies to: 480-480, 719-719, 779-779
936-936
: LGTM: Consistent variable usageThe variable renaming from
n_batches
ton_systems
maintains the same logic flow while using more semantically appropriate terminology.Also applies to: 942-942
974-974
: LGTM: Updated shape annotationThe docstring update from
[n_batches]
to[n_systems]
correctly reflects the tensor shape expectations after the terminology change.
1022-1023
: LGTM: Consistent error handling updatesThe error validation and conditional checks have been updated to use the new
n_systems
attribute, maintaining the same validation logic with improved terminology.Also applies to: 1028-1029, 1060-1060
tests/test_runners.py (5)
152-152
: LGTM: Consistent test updatesThe trajectory file generation logic has been updated to use
n_systems
instead ofn_batches
, maintaining the same test behavior while aligning with the terminology changes.Also applies to: 234-234, 343-343, 417-417
801-801
: LGTM: Updated assertionThe assertion correctly uses the new
n_systems
attribute, ensuring the test validates the expected tensor shape after the terminology change.
809-809
: LGTM: Mock state fixture updatesThe mock state creation has been consistently updated to use
system_idx
instead ofbatch
terminology, maintaining the same test fixture functionality while using more appropriate naming.Also applies to: 815-816, 820-820, 823-823
861-861
: LGTM: Test assertion updatesThe test assertions have been updated to use the new
n_systems
attribute, ensuring proper validation of the state objects after the terminology change.Also applies to: 880-880, 891-891
933-933
: LGTM: Controlled mock state updatesThe controlled mock state creation has been updated to use
system_idx
terminology and related variable names, maintaining the same test logic while using more semantically appropriate naming.Also applies to: 935-936, 940-948
torch_sim/models/interface.py (3)
68-68
: LGTM: Updated documentation examplesThe shape annotations in the docstring examples have been updated from
[n_batches]
to[n_systems]
, maintaining consistency with the terminology changes while providing accurate documentation.Also applies to: 70-70
177-177
: LGTM: Interface specification updatesThe interface documentation has been updated to use
"system_idx"
instead of"batch"
and updated tensor shape annotations to use[n_systems]
. This improves semantic clarity while maintaining the same interface contract.Also applies to: 178-178, 184-184, 186-186
259-259
: LGTM: Validation function updatesThe validation function variable has been renamed from
og_batch
toog_system_idx
with corresponding updates to the validation logic, maintaining the same validation behavior while using consistent terminology.Also applies to: 269-270
torch_sim/io.py (7)
36-36
: LGTM: Updated docstring descriptionsThe docstring descriptions have been updated from "one per batch" to "one per system", providing more semantically accurate documentation for the conversion functions.
Also applies to: 82-82, 138-138
53-53
: LGTM: Consistent variable renaming in state_to_atomsThe variable renaming from
batch
tosystem_idx
and related updates maintain the same conversion logic while using more appropriate terminology. The iteration and masking logic remains functionally identical.Also applies to: 55-55, 58-59, 60-62
101-101
: LGTM: Consistent updates in state_to_structuresThe conversion function has been updated to use
system_idx
terminology with corresponding variable renaming throughout the function, maintaining the same Pymatgen Structure conversion logic.Also applies to: 103-103, 105-106, 109-114
155-155
: LGTM: Consistent updates in state_to_phonopyThe PhonopyAtoms conversion function has been updated to use
system_idx
terminology, maintaining the same conversion logic while using more semantically appropriate variable names.Also applies to: 157-157, 160-164
228-232
: LGTM: Updated atoms_to_state functionThe function has been updated to use
system_idx
terminology in variable names and comments, with the SimState constructor call updated accordingly. The conversion logic remains unchanged.Also applies to: 244-244
300-304
: LGTM: Updated structures_to_state functionThe Pymatgen Structure to SimState conversion has been updated to use
system_idx
terminology, maintaining the same conversion logic while using more appropriate variable names.Also applies to: 312-312
371-375
: LGTM: Updated phonopy_to_state functionThe PhonopyAtoms to SimState conversion has been updated to use
system_idx
terminology in variable names and comments, with the SimState constructor call updated accordingly.Also applies to: 390-390
tests/test_state.py (4)
12-12
: LGTM - Import updated correctlyThe import statement correctly updates from
_normalize_batch_indices
to_normalize_system_indices
to match the function rename.
31-37
: LGTM - Property scope assertions updated correctlyThe test assertions correctly reflect the rename from batch-related properties to system-related properties (
system_idx
andper_system
scope).
409-463
: LGTM - Function rename and test logic are correctThe test function has been properly renamed from
test_normalize_batch_indices
totest_normalize_system_indices
and all function calls and assertions have been updated consistently. The test logic remains intact while using the new terminology.
623-646
: LGTM - Excellent backward compatibility testThis new test ensures that deprecated batch properties continue to work correctly and are properly aliased to the new system properties. The test covers both getter and setter behavior, which is crucial for maintaining API compatibility.
torch_sim/trajectory.py (2)
211-242
: LGTM - Clear documentation and validation logic updatesThe documentation and validation logic have been correctly updated to use system terminology. The validation logic properly checks
state.n_systems
against the number of trajectory files, maintaining the same functional behavior.
676-725
: LGTM - Parameter rename is consistent and well-documentedThe parameter rename from
batch_index
tosystem_index
is consistent throughout the method signature, documentation, and implementation. The logic for handling system indices remains functionally identical.torch_sim/models/mace.py (4)
94-94
: LGTM - Attribute documentation updated correctlyThe attribute documentation properly reflects the rename from
batch
tosystem_idx
while maintaining the same semantic meaning.
115-135
: LGTM - Constructor parameter rename is consistentThe constructor parameter has been correctly renamed from
batch
tosystem_idx
with corresponding documentation updates. The parameter meaning and usage remain identical.
197-199
: LGTM - Method name and signature updated appropriatelyThe method name change from
setup_from_batch
tosetup_from_batch
and parameter rename tosystem_idx
accurately reflects the new terminology while maintaining the same functionality.
337-337
: LGTM - Model interface maintained correctlyThe call to the underlying MACE model continues to use the
batch
parameter name as expected by the MACE interface, while the internal TorchSim code uses the clearersystem_idx
terminology.torch_sim/monte_carlo.py (4)
44-56
: LGTM - Documentation updated to reflect system-based swapsThe documentation correctly explains that swaps occur within the same system, and the return type documentation properly reflects that swaps are generated per system.
57-104
: LGTM - System indexing logic is correctThe variable renames from
batch
tosystem
are consistent throughout the function. The logic for generating swaps within systems remains functionally identical, with proper handling of system boundaries and indices.
127-141
: LGTM - Validation function updated correctlyThe validation function parameter and logic have been properly updated to use
system_idx
terminology. The validation ensures swaps only occur within the same system, which is the correct behavior.
236-236
: LGTM - State initialization and validation updatedThe state initialization and validation calls have been properly updated to use
system_idx
instead ofbatch
, maintaining the same functionality with clearer terminology.Also applies to: 268-268
torch_sim/quantities.py (5)
27-40
: LGTM - Parameter rename and documentation updated correctlyThe parameter rename from
batch
tosystem_idx
is consistent and the documentation clearly explains that it indicates system membership of each particle. The terminology better reflects the semantic meaning.
54-69
: LGTM - System-based temperature calculation logic is correctThe calculation logic has been properly updated to use system terminology while maintaining the same mathematical operations. The bincount and segment_reduce operations correctly group by system indices.
76-92
: LGTM - Function signature and documentation updated consistentlyThe
calc_temperature
function properly forwards the renamed parameter tocalc_kT
, maintaining the same interface with clearer terminology.
100-130
: LGTM - Kinetic energy calculation updated correctlyThe kinetic energy calculation function has been properly updated to use system terminology in parameters and documentation. The logic for per-system calculations remains mathematically correct.
144-159
: LGTM - Batch-wise function correctly renamed to system-wiseThe function rename from computing maximum force per batch to per system is semantically correct. The implementation properly uses
state.system_idx
andstate.n_systems
to group forces by system.torch_sim/integrators/md.py (4)
24-34
: LGTM! Terminology improvements enhance code clarity.The renaming from "batch" to "system" terminology provides better semantic clarity about what these tensors represent. The shape annotations and attribute names are consistently updated throughout the docstring.
54-68
: LGTM! Parameter renaming improves semantic clarity.The parameter name change from
batch
tosystem_idx
and corresponding docstring updates make the function signature more descriptive and consistent with the broader terminology changes.
82-107
: LGTM! Implementation consistently uses new terminology.The variable renaming from
batchwise_momenta
tosystemwise_momenta
and consistent use ofsystem_idx
throughout the function maintains the same logic while improving code readability.Note: Static analysis indicates some lines (83, 90, 95-96) lack test coverage. Consider adding tests for these conditional branches to ensure the temperature indexing logic is properly tested.
121-152
: LGTM! Docstring updates and function calls are consistent.The shape annotations in docstrings are properly updated from
[n_batches]
to[n_systems]
, and the function call topbc_wrap_batched
correctly uses the newsystem_idx
attribute.torch_sim/integrators/nvt.py (6)
48-52
: LGTM! Docstring shape annotations are consistently updated.The shape annotations are properly updated from
[n_batches]
to[n_systems]
throughout the function documentation, maintaining consistency with the broader terminology changes.
96-122
: LGTM! Function consistently uses new system indexing.The docstring and implementation are properly updated to use
state.system_idx
instead ofstate.batch
, maintaining the same indexing logic while improving code clarity.
150-181
: LGTM! Function calls and object construction use consistent terminology.The
calculate_momenta
function call andMDState
constructor are properly updated to usesystem_idx
instead ofbatch
, maintaining functionality while improving code clarity.
205-209
: LGTM! Docstring consistently updated.The shape annotations in the docstring are properly updated from
[n_batches]
to[n_systems]
, maintaining consistency with the terminology changes throughout the codebase.
366-380
: LGTM! Function consistently uses system terminology.The function call to
calculate_momenta
and the subsequent calculations are properly updated to usesystem_idx
and system-related terminology, maintaining the same logic while improving code clarity.
434-483
: LGTM! Functions consistently use system terminology.The
calc_kinetic_energy
function calls and related calculations are properly updated to usesystem_idx
and system-related terminology, maintaining the same computational logic while improving code readability.torch_sim/transforms.py (4)
26-59
: LGTM! Improved terminology enhances code clarity.The shape annotations and error messages are updated to use "system" terminology instead of "batch", providing better semantic clarity about what these dimensions represent. The error message is also more descriptive about the requirements for multiple systems.
158-172
: LGTM! Function signature and docstring are consistently updated.The parameter name change from
batch
tosystem_idx
and the corresponding docstring updates make the function signature more descriptive and consistent with the broader terminology changes.
185-211
: LGTM! Implementation consistently uses new terminology.The variable names and comments are properly updated to use system terminology (
unique_systems
,n_systems
,system_idx
) while maintaining the same computational logic. The code is more readable with the improved naming.
538-1037
: LGTM! All remaining functions consistently use new terminology.The functions
compute_cell_shifts
,build_naive_neighborhood
, andbuild_linked_cell_neighborhood
are properly updated to usesystem_mapping
instead ofbatch_mapping
in parameter names, variable names, and comments. The computational logic remains unchanged while improving code clarity.torch_sim/integrators/npt.py (3)
22-74
: LGTM! Comprehensive docstring updates for system terminology.The NPTLangevinState class documentation has been thoroughly updated to reflect the batch→system renaming, with correct tensor shape annotations.
109-139
: Consider adding test coverage for system-specific cell force calculations.The implementation correctly uses system-based indexing, but static analysis indicates these critical lines lack test coverage:
- Volume calculation per system (line 109)
- Pressure tensor expansion (line 120)
- Temperature expansion (line 132)
- Scaling by atoms per system (line 139)
These calculations are essential for NPT dynamics correctness.
806-898
: Well-structured NPTNoseHooverState with proper system dimensions.The class correctly implements system-based indexing with appropriate tensor shapes for cell variables. The logarithmic cell coordinate parameterization is clearly documented.
torch_sim/state.py (3)
25-164
: Excellent refactoring of SimState to use system terminology.The core class has been thoroughly updated with:
- Clear documentation using system-based terminology
- Proper validation of system indices as consecutive integers
- Correct tensor shape expectations
620-639
: Excellent implementation of system index remapping.The code correctly handles the remapping of system indices to ensure they remain consecutive after filtering. This is crucial for maintaining the invariant that system indices must be consecutive integers starting from 0.
809-896
: Well-implemented state concatenation with proper system index management.The concatenate_states function correctly:
- Maintains system index continuity across concatenated states
- Handles per-system properties appropriately
- Updates indices with proper offsets
14a9b23
to
c335511
Compare
add .vscode to gitignore rename per_batch to per_graph wip convert many batches to graphs convert more batches to graphs rename all of batch indices rename all of batch indices wip fix tests fix more tests more test renaming more test renaming again fix test_deform_grad_batched rename more batch to use graphs update mock state to use graphs precommit fix bug rename in calculate_momenta fix more precommit rename more batch to graphs more renames more renaming fix deprecated text in docstring added test for deprecated batch properties in simstate rename batch to graph more renames cleanup minor typo code rabbit reviewgs rename batch to system bump minor version
c335511
to
9a1b87e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
torch_sim/integrators/npt.py (1)
498-502
: Redundant L_n shape check remains from previous review.This conditional recalculation of L_n is unnecessary as identified in the previous review. The check is redundant because L_n is always freshly computed with the correct shape at lines 751-753 immediately before calling
langevin_position_step
.torch_sim/state.py (1)
165-219
: Deprecated properties properly implemented with warnings.The deprecated properties
n_atoms_per_batch
,batch
, andn_batches
have been correctly implemented withDeprecationWarning
warnings and proper redirection to the new properties. This addresses the past review comment requesting these warnings.
🧹 Nitpick comments (1)
torch_sim/state.py (1)
122-126
: Consider improving system index validation logic.The current validation logic uses
torch.unique_consecutive
andtorch.bincount
to ensure system indices are unique consecutive integers. However, the TODO comment indicates this approach may not be reliable.Consider implementing a more robust validation approach:
-# TODO(curtis): I feel like this logic is not reliable. -# I'll come up with something better later. -_, counts = torch.unique_consecutive(self.system_idx, return_counts=True) -if not torch.all(counts == torch.bincount(self.system_idx)): - raise ValueError("System indices must be unique consecutive integers") +# Validate system indices are consecutive integers starting from 0 +sorted_indices = torch.sort(self.system_idx)[0] +expected_indices = torch.arange(self.n_systems, device=self.device, dtype=torch.int64) +expected_indices = expected_indices.repeat_interleave(torch.bincount(self.system_idx)) +if not torch.equal(sorted_indices, expected_indices): + raise ValueError("System indices must be unique consecutive integers starting from 0")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (68)
.gitignore
(1 hunks)examples/scripts/1_Introduction/1.2_MACE.py
(1 hunks)examples/scripts/2_Structural_optimization/2.3_MACE_Gradient_Descent.py
(1 hunks)examples/scripts/2_Structural_optimization/2.5_MACE_UnitCellFilter_Gradient_Descent.py
(1 hunks)examples/scripts/2_Structural_optimization/2.6_MACE_UnitCellFilter_FIRE.py
(1 hunks)examples/scripts/2_Structural_optimization/2.7_MACE_FrechetCellFilter_FIRE.py
(1 hunks)examples/scripts/3_Dynamics/3.10_Hybrid_swap_mc.py
(1 hunks)examples/scripts/3_Dynamics/3.11_Lennard_Jones_NPT_Langevin.py
(2 hunks)examples/scripts/3_Dynamics/3.12_MACE_NPT_Langevin.py
(5 hunks)examples/scripts/3_Dynamics/3.13_MACE_NVE_non_pbc.py
(1 hunks)examples/scripts/3_Dynamics/3.2_MACE_NVE.py
(1 hunks)examples/scripts/3_Dynamics/3.3_MACE_NVE_cueq.py
(1 hunks)examples/scripts/3_Dynamics/3.4_MACE_NVT_Langevin.py
(1 hunks)examples/scripts/3_Dynamics/3.5_MACE_NVT_Nose_Hoover.py
(1 hunks)examples/scripts/3_Dynamics/3.6_MACE_NVT_Nose_Hoover_temp_profile.py
(1 hunks)examples/scripts/3_Dynamics/3.7_Lennard_Jones_NPT_Nose_Hoover.py
(2 hunks)examples/scripts/3_Dynamics/3.8_MACE_NPT_Nose_Hoover.py
(4 hunks)examples/scripts/3_Dynamics/3.9_MACE_NVT_staggered_stress.py
(1 hunks)examples/scripts/4_High_level_api/4.2_auto_batching_api.py
(1 hunks)examples/scripts/5_Workflow/5.2_In_Flight_WBM.py
(1 hunks)examples/scripts/7_Others/7.3_Batched_neighbor_list.py
(2 hunks)examples/scripts/7_Others/7.6_Compare_ASE_to_VV_FIRE.py
(10 hunks)examples/tutorials/autobatching_tutorial.py
(2 hunks)examples/tutorials/high_level_tutorial.py
(1 hunks)examples/tutorials/hybrid_swap_tutorial.py
(1 hunks)examples/tutorials/low_level_tutorial.py
(1 hunks)examples/tutorials/state_tutorial.py
(6 hunks)pyproject.toml
(1 hunks)tests/models/conftest.py
(1 hunks)tests/test_autobatching.py
(6 hunks)tests/test_correlations.py
(1 hunks)tests/test_integrators.py
(6 hunks)tests/test_io.py
(6 hunks)tests/test_monte_carlo.py
(5 hunks)tests/test_neighbors.py
(3 hunks)tests/test_optimizers.py
(9 hunks)tests/test_runners.py
(9 hunks)tests/test_state.py
(18 hunks)tests/test_trajectory.py
(5 hunks)tests/test_transforms.py
(10 hunks)torch_sim/autobatching.py
(12 hunks)torch_sim/integrators/md.py
(7 hunks)torch_sim/integrators/npt.py
(51 hunks)torch_sim/integrators/nve.py
(5 hunks)torch_sim/integrators/nvt.py
(10 hunks)torch_sim/io.py
(12 hunks)torch_sim/math.py
(1 hunks)torch_sim/models/fairchem.py
(1 hunks)torch_sim/models/graphpes.py
(1 hunks)torch_sim/models/interface.py
(4 hunks)torch_sim/models/lennard_jones.py
(2 hunks)torch_sim/models/mace.py
(9 hunks)torch_sim/models/metatomic.py
(2 hunks)torch_sim/models/morse.py
(2 hunks)torch_sim/models/orb.py
(3 hunks)torch_sim/models/particle_life.py
(2 hunks)torch_sim/models/sevennet.py
(2 hunks)torch_sim/models/soft_sphere.py
(4 hunks)torch_sim/monte_carlo.py
(5 hunks)torch_sim/neighbors.py
(3 hunks)torch_sim/optimizers.py
(38 hunks)torch_sim/quantities.py
(5 hunks)torch_sim/runners.py
(8 hunks)torch_sim/state.py
(23 hunks)torch_sim/trajectory.py
(6 hunks)torch_sim/transforms.py
(15 hunks)torch_sim/typing.py
(1 hunks)torch_sim/workflows/a2c.py
(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- examples/scripts/5_Workflow/5.2_In_Flight_WBM.py
- pyproject.toml
- examples/tutorials/high_level_tutorial.py
- examples/scripts/2_Structural_optimization/2.7_MACE_FrechetCellFilter_FIRE.py
- examples/tutorials/state_tutorial.py
- torch_sim/optimizers.py
🚧 Files skipped from review as they are similar to previous changes (58)
- examples/scripts/2_Structural_optimization/2.6_MACE_UnitCellFilter_FIRE.py
- .gitignore
- examples/scripts/3_Dynamics/3.9_MACE_NVT_staggered_stress.py
- examples/scripts/4_High_level_api/4.2_auto_batching_api.py
- examples/scripts/3_Dynamics/3.2_MACE_NVE.py
- examples/scripts/2_Structural_optimization/2.5_MACE_UnitCellFilter_Gradient_Descent.py
- torch_sim/math.py
- examples/scripts/3_Dynamics/3.11_Lennard_Jones_NPT_Langevin.py
- tests/models/conftest.py
- torch_sim/typing.py
- examples/tutorials/low_level_tutorial.py
- examples/tutorials/autobatching_tutorial.py
- torch_sim/models/orb.py
- torch_sim/models/sevennet.py
- examples/scripts/3_Dynamics/3.6_MACE_NVT_Nose_Hoover_temp_profile.py
- torch_sim/workflows/a2c.py
- tests/test_autobatching.py
- torch_sim/models/graphpes.py
- examples/scripts/3_Dynamics/3.4_MACE_NVT_Langevin.py
- tests/test_integrators.py
- torch_sim/integrators/nve.py
- torch_sim/neighbors.py
- tests/test_optimizers.py
- tests/test_correlations.py
- examples/scripts/7_Others/7.6_Compare_ASE_to_VV_FIRE.py
- tests/test_trajectory.py
- examples/scripts/3_Dynamics/3.13_MACE_NVE_non_pbc.py
- examples/scripts/3_Dynamics/3.3_MACE_NVE_cueq.py
- torch_sim/models/lennard_jones.py
- torch_sim/models/fairchem.py
- examples/scripts/3_Dynamics/3.10_Hybrid_swap_mc.py
- tests/test_neighbors.py
- torch_sim/io.py
- examples/tutorials/hybrid_swap_tutorial.py
- examples/scripts/2_Structural_optimization/2.3_MACE_Gradient_Descent.py
- examples/scripts/3_Dynamics/3.7_Lennard_Jones_NPT_Nose_Hoover.py
- torch_sim/models/metatomic.py
- torch_sim/quantities.py
- examples/scripts/3_Dynamics/3.8_MACE_NPT_Nose_Hoover.py
- examples/scripts/3_Dynamics/3.12_MACE_NPT_Langevin.py
- examples/scripts/1_Introduction/1.2_MACE.py
- tests/test_monte_carlo.py
- examples/scripts/7_Others/7.3_Batched_neighbor_list.py
- tests/test_runners.py
- examples/scripts/3_Dynamics/3.5_MACE_NVT_Nose_Hoover.py
- torch_sim/runners.py
- tests/test_io.py
- torch_sim/trajectory.py
- tests/test_transforms.py
- torch_sim/models/morse.py
- torch_sim/models/particle_life.py
- torch_sim/models/mace.py
- torch_sim/models/soft_sphere.py
- torch_sim/monte_carlo.py
- torch_sim/models/interface.py
- torch_sim/transforms.py
- tests/test_state.py
- torch_sim/integrators/nvt.py
🧰 Additional context used
🧬 Code Graph Analysis (2)
torch_sim/autobatching.py (1)
torch_sim/state.py (2)
n_systems
(222-224)concatenate_states
(829-915)
torch_sim/integrators/npt.py (7)
torch_sim/state.py (8)
device
(142-144)dtype
(147-149)n_systems
(222-224)n_atoms_per_system
(157-163)volume
(227-229)momenta
(380-386)to
(341-355)clone
(259-275)torch_sim/models/interface.py (4)
device
(107-109)device
(112-116)dtype
(119-121)dtype
(124-128)tests/conftest.py (2)
device
(24-25)dtype
(29-30)tests/test_neighbors.py (1)
dtype
(15-16)torch_sim/integrators/md.py (2)
velocities
(44-48)calculate_momenta
(51-109)torch_sim/integrators/nvt.py (1)
velocities
(269-273)torch_sim/quantities.py (1)
calc_kinetic_energy
(96-130)
🪛 GitHub Check: codecov/patch
torch_sim/autobatching.py
[warning] 1023-1023: torch_sim/autobatching.py#L1023
Added line #L1023 was not covered by tests
[warning] 1029-1029: torch_sim/autobatching.py#L1029
Added line #L1029 was not covered by tests
torch_sim/integrators/md.py
[warning] 83-83: torch_sim/integrators/md.py#L83
Added line #L83 was not covered by tests
[warning] 90-90: torch_sim/integrators/md.py#L90
Added line #L90 was not covered by tests
[warning] 95-96: torch_sim/integrators/md.py#L95-L96
Added lines #L95 - L96 were not covered by tests
torch_sim/integrators/npt.py
[warning] 246-246: torch_sim/integrators/npt.py#L246
Added line #L246 was not covered by tests
[warning] 249-249: torch_sim/integrators/npt.py#L249
Added line #L249 was not covered by tests
[warning] 286-286: torch_sim/integrators/npt.py#L286
Added line #L286 was not covered by tests
[warning] 294-294: torch_sim/integrators/npt.py#L294
Added line #L294 was not covered by tests
[warning] 511-511: torch_sim/integrators/npt.py#L511
Added line #L511 was not covered by tests
[warning] 514-514: torch_sim/integrators/npt.py#L514
Added line #L514 was not covered by tests
[warning] 585-585: torch_sim/integrators/npt.py#L585
Added line #L585 was not covered by tests
[warning] 588-588: torch_sim/integrators/npt.py#L588
Added line #L588 was not covered by tests
[warning] 891-893: torch_sim/integrators/npt.py#L891-L893
Added lines #L891 - L893 were not covered by tests
[warning] 966-968: torch_sim/integrators/npt.py#L966-L968
Added lines #L966 - L968 were not covered by tests
[warning] 979-979: torch_sim/integrators/npt.py#L979
Added line #L979 was not covered by tests
[warning] 1017-1017: torch_sim/integrators/npt.py#L1017
Added line #L1017 was not covered by tests
[warning] 1020-1021: torch_sim/integrators/npt.py#L1020-L1021
Added lines #L1020 - L1021 were not covered by tests
[warning] 1089-1089: torch_sim/integrators/npt.py#L1089
Added line #L1089 was not covered by tests
[warning] 1154-1154: torch_sim/integrators/npt.py#L1154
Added line #L1154 was not covered by tests
[warning] 1212-1212: torch_sim/integrators/npt.py#L1212
Added line #L1212 was not covered by tests
[warning] 1216-1216: torch_sim/integrators/npt.py#L1216
Added line #L1216 was not covered by tests
[warning] 1219-1224: torch_sim/integrators/npt.py#L1219-L1224
Added lines #L1219 - L1224 were not covered by tests
[warning] 1234-1234: torch_sim/integrators/npt.py#L1234
Added line #L1234 was not covered by tests
[warning] 1273-1275: torch_sim/integrators/npt.py#L1273-L1275
Added lines #L1273 - L1275 were not covered by tests
[warning] 1288-1289: torch_sim/integrators/npt.py#L1288-L1289
Added lines #L1288 - L1289 were not covered by tests
[warning] 1413-1413: torch_sim/integrators/npt.py#L1413
Added line #L1413 was not covered by tests
[warning] 1417-1418: torch_sim/integrators/npt.py#L1417-L1418
Added lines #L1417 - L1418 were not covered by tests
[warning] 1425-1425: torch_sim/integrators/npt.py#L1425
Added line #L1425 was not covered by tests
[warning] 1428-1429: torch_sim/integrators/npt.py#L1428-L1429
Added lines #L1428 - L1429 were not covered by tests
[warning] 1438-1438: torch_sim/integrators/npt.py#L1438
Added line #L1438 was not covered by tests
[warning] 1448-1448: torch_sim/integrators/npt.py#L1448
Added line #L1448 was not covered by tests
[warning] 1545-1545: torch_sim/integrators/npt.py#L1545
Added line #L1545 was not covered by tests
🔇 Additional comments (26)
torch_sim/autobatching.py (8)
237-237
: LGTM: Consistent terminology update in print statement.The print statement now correctly uses "systems" instead of "batches" to align with the new terminology.
296-297
: LGTM: Variable renaming improves clarity.The variable name change from
n_batches
ton_systems
better reflects the actual concept being represented.
346-347
: LGTM: Property access updated correctly.The change from
n_batches
ton_systems
is consistent with the SimState class property renaming.
408-410
: LGTM: Print statement terminology consistently updated.The print statement now correctly references "systems" instead of "batches" in both contexts, maintaining consistency with the new terminology.
431-431
: LGTM: Docstring terminology updated consistently.The docstring updates correctly change "batch" to "system" throughout the parameter descriptions, improving clarity about what the code actually handles.
Also applies to: 480-480, 719-719, 779-779
936-942
: LGTM: Variable names updated consistently.The variable name change from
n_batches
ton_systems
is consistent with the overall terminology update and maintains the same logic.
1022-1023
: LGTM: Error messages updated with consistent terminology.The error messages now correctly reference
n_systems
instead ofn_batches
, maintaining consistency with the updated property names.Also applies to: 1028-1029
1060-1060
: LGTM: Conditional check updated correctly.The property access is correctly updated to use
n_systems
instead ofn_batches
.torch_sim/integrators/md.py (7)
24-24
: LGTM: MDState class documentation updated consistently.The tensor shape annotations and attribute descriptions now correctly use "system" terminology instead of "batch", improving clarity about what the class represents.
Also applies to: 27-27, 29-29, 34-34
54-54
: LGTM: Function signature and docstring updated correctly.The parameter rename from
batch
tosystem_idx
and the corresponding docstring updates improve clarity about the indexing scheme.Also applies to: 67-68
82-83
: LGTM: Code logic updated with consistent terminology.The comment and indexing operation correctly use
system_idx
instead ofbatch
, maintaining the same functionality with clearer terminology.
90-91
: LGTM: Variable names updated consistently throughout function.The variable names are systematically updated to use "system" terminology while maintaining the same computational logic for momenta calculation.
Also applies to: 94-96
107-107
: LGTM: Indexing operation updated correctly.The indexing operation now uses
system_idx
instead ofbatch
, maintaining consistency with the parameter renaming.
121-121
: LGTM: Docstring tensor shape annotations updated.The docstring comments now correctly reference
[n_systems]
instead of[n_batches]
for the timestep tensor shape.Also applies to: 141-141
150-152
: LGTM: PBC wrapping function call updated correctly.The function call now uses
state.system_idx
instead ofstate.batch
, maintaining consistency with the attribute renaming in the MDState class.torch_sim/integrators/npt.py (1)
1-1673
: Excellent systematic refactoring of batch-to-system terminology.The renaming from "batch" to "system" terminology throughout this NPT integrator is comprehensive and well-executed. The changes consistently update:
- Class attributes and properties (
n_batches
→n_systems
,batch
→system_idx
)- Function parameters and variable names
- Docstrings and comments to reflect the new semantics
- Tensor shape annotations (
[n_batches]
→[n_systems]
)The mathematical logic and algorithmic implementation remain unchanged, which is exactly what's needed for this semantic refactoring. This aligns well with the PR objective to clarify that "batch" actually refers to "systems" in the codebase.
torch_sim/state.py (10)
32-84
: Systematic documentation and attribute renaming looks good.The documentation updates throughout the class docstring and attribute definitions are consistent and accurately reflect the conceptual shift from "batch" to "system" terminology. The attribute renaming from
batch
tosystem_idx
maintains semantic clarity.
312-373
: Method parameter renaming is consistent.The parameter renaming in methods
pop
and__getitem__
frombatch_indices
tosystem_indices
is consistent with the overall refactoring and maintains the same functionality.
420-457
: Function renaming and implementation is correct.The function
_normalize_batch_indices
has been properly renamed to_normalize_system_indices
with all parameter names, documentation, and internal logic updated accordingly. The function handles various input types correctly.
496-574
: Property scope inference updates are accurate.The
infer_property_scope
function has been correctly updated to use "per_system" instead of "per_batch" terminology throughout, including parameter names, documentation, and return type annotations.
613-666
: Attribute filtering logic properly updated.The
_filter_attrs_by_mask
function has been correctly updated to handle "per_system" attributes instead of "per_batch", including the system index remapping logic for maintaining consecutive indices.
669-724
: State splitting logic correctly refactored.The
_split_state
function has been properly updated to use "system" terminology throughout, including variable names, comments, and logic for handling per-system attributes.
727-779
: State popping implementation is consistent.The
_pop_states
function has been correctly updated with "system" terminology, including parameter names, documentation, and the logic for creating system masks.
782-826
: State slicing functionality properly updated.The
_slice_state
function has been correctly refactored to use "system" terminology throughout, including parameter names, documentation, and the implementation logic.
829-915
: State concatenation logic is correctly updated.The
concatenate_states
function has been properly updated to use "system" terminology, including variable names, comments, and the logic for handling per-system properties and system index offsets.
946-950
: State initialization validation updated correctly.The validation logic in
initialize_state
has been properly updated to check forn_systems == 1
instead ofn_batches == 1
, maintaining the same functionality with the new terminology.
I think this is broadly a good change from a usability and intuitiveness perspective, thanks @curtischong. I couldn't find any missed The only tension I see arises from how batch is used in other contexts. Nequip, for example, uses The question is then, "to what extent do we adopt prior norms vs make choices that improve usability for the materials community?" In this case, I think |
Mace also uses something similar. However, I also think it's fine for MLIPs to not follow this convention right now because they are more like individual repos with few dependents. I think for a framework, it's more important to have variable names that accurately reflect what they are. I think we should do it because the surface area of using "systems" in our codebase is might wider. Additionally, we have a higher chance of ambiguity (due to the autobatchers). |
In the context of ML potentials, |
I agree that we should not be disruptive. But I also think that renaming it as system really helps the readability in our codebase. How about I make the In an upcoming PR I will also make it so that we'll have multiple constructors for |
I do not have a strong opinion on the suggestions you made. As I don't have much experience in maintaining packages, I will leave this decision to others. |
I think this is a good middle ground, would be nice to not immediately break some workflows. Let's just throw a deprecation notice and pull the batch getters/setters in a month.
If we are going to deprecate I don't see too much of a need to maintain a separate constructor long term. I suspect most people are instantiating from pymatgen or ase anyway. |
Summary
This change is quite involved. But I didn't want to split it up into multiple parts because "batch" is so pervasive within the codebase. But it feels like the right thing to do. Notable, I changed these interfaces in the SimState:
The changes to getitem and init will cause breaking changes, but I feel like it is worth it while torchsim's adoption is currently minimal.
I have also bumped the minor version from 0.2.2 to 0.3.0
Notes of my changes
Note: this PR is an improved version of this PR: #216 (with Rhys' suggestions)
Summary by CodeRabbit
Refactor
Documentation
Tests
Chores
.gitignore
to exclude.vscode/
directory.