feat(core): support dynamic dt by reading from scene config live - #454
feat(core): support dynamic dt by reading from scene config live#454MuGdxy 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>
There was a problem hiding this comment.
Code Review
This pull request implements dynamic time-step support across the CUDA backend by replacing static dt members with dt_attr attribute slots. This change allows simulation systems to react to time-step modifications in the scene configuration during runtime. The PR includes comprehensive test cases verifying the dynamic behavior. My feedback focuses on performance optimizations, specifically caching the dt value before entering hot loops to avoid redundant attribute lookups, and improving robustness by adding assertions to ensure the dt attribute is successfully retrieved from the configuration.
| for(auto constraint : m_impl.constraints.view()) | ||
| { | ||
| 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()}; | ||
| constraint->compute_energy(this_info); | ||
| } |
There was a problem hiding this comment.
For better performance, consider caching the dt value from the attribute slot once before entering the loop, rather than re-accessing the view and indexing it in every iteration. This aligns with the practice of passing pre-loaded data to functors in hot loops.
Float dt = m_impl.dt_attr->view()[0];
for(auto constraint : m_impl.constraints.view())
{
ComputeEnergyInfo this_info{
&m_impl, constraint->m_index, dt, info.energies()};
constraint->compute_energy(this_info);
}
References
- In performance-critical CUDA code, pass pre-loaded data as parameters to functors within hot loops to improve performance.
| { | ||
| EnergyInfo this_info{this, c->m_index, dt, info.energies()}; | ||
| EnergyInfo this_info{this, c->m_index, dt_attr->view()[0], info.energies()}; | ||
| c->compute_energy(this_info); | ||
| } |
There was a problem hiding this comment.
Consider caching the dt value before the loop to avoid redundant view access and indexing on every iteration. This ensures that data is pre-loaded and passed as a parameter to the functor, improving performance in hot loops.
Float dt = dt_attr->view()[0];
for(auto&& [i, c] : enumerate(constitution_view))
{
EnergyInfo this_info{this, c->m_index, dt, info.energies()};
c->compute_energy(this_info);
}
References
- In performance-critical CUDA code, pass pre-loaded data as parameters to functors within hot loops to improve performance.
| { | ||
| auto config = world.scene().config(); | ||
| dt = config.find<Float>("dt")->view()[0]; | ||
| dt_attr = config.find<Float>("dt"); |
There was a problem hiding this comment.
It is safer to verify that the "dt" attribute slot was successfully found before proceeding, as subsequent calls to view() on a null pointer will cause a crash. Adding an assertion here would improve robustness.
dt_attr = config.find<Float>("dt");
UIPC_ASSERT(dt_attr, "dt attribute not found in scene config");
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