Changes for Sumo#116
Conversation
slecleach
left a comment
There was a problem hiding this comment.
Thanks for the Sumo backend work — I walked through the diff in detail and found two behavioral regressions to fix before merge:
SimulationNode._init_simcan now fail for cfg-registered custom tasks
- In
judo/app/dora/simulation.py,_init_sim()callsget_registered_tasks().get(task_name)before any backend instance is constructed. - Custom tasks are registered in
Simulation.__init__viaregister_tasks_from_cfg(task_registration_cfg), which only runs whensim_backend_cls(...)is instantiated. - That means
init_taskvalues supplied only throughtask_registration_cfgnow raiseValueErrorduring node startup.
- Switching task resets controller state/config unexpectedly
- In
judo/app/dora/controller.py,update_task()now rebuilds the controller viaself._make_controller_fn(init_task=..., init_optimizer=...). - This path does not preserve runtime state/config that previously survived task switches (notably controller config overrides like horizon/control_freq), since
make_controller()creates a freshControllerConfig. - It also omits the node's original registration/customization args (
task_registration_cfg,optimizer_registration_cfg, and any custom kwargs), so behavior can drift after a task change.
Please preserve existing controller/task/optimizer config where intended and ensure task registration from cfg occurs before task lookup in simulation init.
I see the concern in point 1, but we never supply the For part 2, we don't mind the overrides being overwritten due selecting a new task. We also only ever reference the optimizer as one source of truth due to a change I made in the API, meaning that any new controller automatically references its optimizer config as the sole source of truth. The behaviour can drift if the configs are changed, but the configs aren't meant to change in between runs. This could be an issue for |
# Conflicts: # judo/controller/controller.py # judo/utils/hierarchical_mj_rollout_backend.py # pixi.lock # pyproject.toml
bhung-bdai
left a comment
There was a problem hiding this comment.
Slight comments
Refactor task registration and backend selection to enable support for Sumo and other C++ backend rollouts.
Task Registry
Replaces the previous
(task_cls, task_config_cls)tuple registry with aTaskRegistrationfrozendataclassthat carriesrollout_backend,simulation_backend, andlocomotion_policy_pathalongside the task and config types. Addsget_task_registration()for direct lookup. Spot tasks are now registered withrollout_backend="mujoco_hierarchical"andsimulation_backend="mujoco_hierarchical"along with their policy paths, removing the need for tasks to exposelocomotion_policy_pathas a property.Rollout & Simulation Backend Renaming
Renames
PolicyMJRolloutBackend→HierarchicalMJRolloutBackendandPolicyMJSimulation→HierarchicalMJSimulationto better reflect their general-purpose hierarchical policy layer rather than a Spot-specific locomotion policy. UpdatesDEFAULT_ROLLOUT_BACKEND_REGISTRYandDEFAULT_SIMULATION_BACKEND_REGISTRYaccordingly, replacing"mujoco_policy"with"mujoco_hierarchical".Backend Resolution via Registry
ControllerNodeandSimulationNodenow resolve their backends by looking up the task's registeredrollout_backend/simulation_backendname at construction time, removing the previous auto-upgrade logic (uses_locomotion_policychecks andsimulation_backendconfig key). Both nodes accept injectable registries and factory functions for extensibility.RenderPose Separation
Splits
xposandxquatout ofMujocoStateinto a newRenderPosedataclass. The simulation node publishes a dedicatedrender_poseoutput topic, and the visualization node subscribes torender_poseinstead of states.Dora Node File Renames
Renames
controller.py→controller_node.py,simulation.py→simulation_node.py, andvisualization.py→visualization_node.pywith updated target paths injudo_dora_default.yaml.mujoco_extensions Import Guard
Adds
_require_mujoco_extensions()to fail fast at CLI entry points (app()andbenchmark()) if the native extension is unavailable, surfacing a clear build instruction rather than a deferred import error.