Remove StateDict type and ensure_sim_state helper#487
Open
orionarcher wants to merge 2 commits intomainfrom
Open
Remove StateDict type and ensure_sim_state helper#487orionarcher wants to merge 2 commits intomainfrom
orionarcher wants to merge 2 commits intomainfrom
Conversation
StateDict = dict was a type alias allowing raw dicts to be passed as state to integrators, optimizers, and models. All callers now pass a proper SimState, so remove the alias, the ensure_sim_state conversion helper, and all StateDict | SimState union annotations across integrators, optimizers, models, the interface ABC, tests, and examples.
1 task
Member
|
this is a pain point of moving to externalize, the upstream nequip has a dep on StateDict which causes the tests to fail. This will impact Nequip, SevenNet and Orb (@cw-tan, @YutackPark, @vsimkus) which have already been externalized. Sorry. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Orion Summary
state: SimState | StateDictis a longstanding compromise in the function signature. As we add more robust typing, I think it's time to fully embraceSimStateas the core object. This PR swaps fromstate: SimState | StateDicttostate: SimStateacross the board. (already got agreement from @abhijeetgangan)Upstream Nequip issue fixed here. Fairchem issue is a flake.
Claude Summary
StateDict = dicttype alias andStateKeyfromtorch_sim/typing.py— these were the only remaining artifacts of the dict-as-state patternensure_sim_stateconversion helper fromtorch_sim/state.py— it's now a pure no-op since all callers pass aSimStatestate: SimState | StateDict→state: SimStateacross all integrators (NVE, NVT, NPT), optimizers (FIRE, BFGS, L-BFGS, gradient descent), models (MACE, ORB, SevenNet, FairChem, FairChem legacy, soft sphere, GraphPES, Morse, LJ, particle life, metatomic, MatterSim), and theModelInterfaceABCSimStatedirectly insteadts.SimState(...)instead of rawdict(...)Test plan
grep -r "StateDict|ensure_sim_state" torch_sim/ tests/returns no matches🤖 Generated with Claude Code