Adding "global", "per_atom", "per_system" features to SimState #224
Replies: 3 comments
-
Note: global is actually a reserved keyword. so we'll probably rename it to "shared". #189 |
Beta Was this translation helpful? Give feedback.
-
I think I've got an idea of how it'll look like. We'll make these attributes dictionaries in SimState "node_features" Then we can add derived classes like MDState. For these derived classes, they store their data inside these attribute dictionaries perhaps important config data should be stored under shared_features. |
Beta Was this translation helpful? Give feedback.
-
Modified from a comment in #227, adding for context: HI @curtischong, this is something I thought about a fair amount and I am happy to revisit. I am not convinced I made the right decision to make everything implicit. I have a few thoughts here:
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
In an effort to properly type the entire library, I'm proposing to add these attributes to the SimState. This way, we don't need to run
infer_property_scope
(https://github.com/Radical-AI/torch-sim/blob/main/torch_sim/state.py#L496) and we automatically know if a property belongs globally to the entire simstate, or per-atom/system.This idea is partially inspired by how orb handles features. They use:
"node_features"
"edge_features"
"system_features"
https://github.com/orbital-materials/orb-models/blob/main/orb_models/forcefield/base.py#L65-L67
Maybe if a state is classified as an "md_state" we'll guarantee exactly which features it'll have? Hopefully by solving this problem, we'll also come up with a good solution to represent md_state.
More discussion in #227, #228
Beta Was this translation helpful? Give feedback.
All reactions