feat(core): support dynamic dt by reading from scene config live - #453
feat(core): support dynamic dt by reading from scene config live#453MuGdxy wants to merge 1 commit into
Conversation
Replace all cached `Float dt` members in ~25 backend systems with
`S<const geometry::AttributeSlot<Float>> dt_attr` so every system reads
dt from the scene config attribute on each use. This allows users to
change dt at runtime via `scene.config().find<Float>("dt")`.
- Add `SceneVisitor::dt()` convenience accessor
- Tolerance checkers now recompute abs_tol dynamically
- Add unit test and sim_case test verifying dynamic dt
Fixes spiriMirror#451
Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements support for dynamic time step (dt) updates by replacing cached dt values with references to the underlying attribute slot across various CUDA backend systems, including affine body dynamics, FEM, and contact systems. This change allows the simulation to respond to dt modifications mid-run. Corresponding tests were added to verify the functionality. Feedback focuses on performance optimizations, specifically suggesting that the dt value be cached in a local variable before entering loops in several animator and constitution manager implementations to avoid redundant attribute slot lookups.
| { | ||
| ComputeEnergyInfo this_info{&m_impl, constraint->m_index, m_impl.dt, info.energies()}; | ||
| ComputeEnergyInfo this_info{ | ||
| &m_impl, constraint->m_index, m_impl.dt_attr->view()[0], info.energies()}; |
There was a problem hiding this comment.
Accessing the attribute slot via view()[0] inside a loop is suboptimal. For better performance and consistency with other systems (e.g., TimeIntegratorManager), consider caching the dt value in a local variable before the loop.
| { | ||
| ComputeEnergyInfo this_info{&m_impl, constraint->m_index, m_impl.dt, info.energies()}; | ||
| ComputeEnergyInfo this_info{ | ||
| &m_impl, constraint->m_index, m_impl.dt_attr->view()[0], info.energies()}; |
There was a problem hiding this comment.
Accessing the attribute slot via view()[0] inside a loop is suboptimal. Consider caching the dt value in a local variable before the loop.
| for(auto&& [i, c] : enumerate(constitution_view)) | ||
| { | ||
| EnergyInfo this_info{this, c->m_index, dt, info.energies()}; | ||
| EnergyInfo this_info{this, c->m_index, dt_attr->view()[0], info.energies()}; |
There was a problem hiding this comment.
Accessing the attribute slot via view()[0] inside a loop is suboptimal. Consider caching the dt value in a local variable before the loop.
| for(auto&& [i, c] : enumerate(constitution_view)) | ||
| { | ||
| EnergyInfo this_info{this, c->m_index, dt, info.energies()}; | ||
| EnergyInfo this_info{this, c->m_index, dt_attr->view()[0], info.energies()}; |
There was a problem hiding this comment.
Accessing the attribute slot via view()[0] inside a loop is suboptimal. Consider caching the dt value in a local variable before the loop.
1 participant
Summary
Replace all cached
Float dtmembers across ~25 CUDA backend systems with live attribute slot references (S<const geometry::AttributeSlot<Float>> dt_attr), enabling users to change the simulation timestep at runtime viascene.config().find<Float>("dt").Previously, each system copied
dtfrom the scene config once duringdo_build()orinit(), making it impossible to change dt dynamically. Now every system reads dt from the scene config attribute on each use, following the same pattern already established bySimEnginefor other config values likem_newton_velocity_tol.Changes
dt()convenience accessor delegating tointernal::Scene::dt()Float dtmember withS<const geometry::AttributeSlot<Float>> dt_attrin Impl structs (time integrator, linear subsystems, line search, contact system, active set, animators, constitution managers, diff reporters)MaxTranslationCheckerandABDToleranceCheckernow recomputeabs_tol = factor * dtdynamically indo_check()instead of caching it indo_build()SceneVisitor::dt(), and sim_case test (93_dynamic_dt) verifying end-to-end that changing dt at runtime affects simulation behaviorFixes #451