Skip to content

Changes for Sumo#116

Open
bhung-bdai wants to merge 12 commits intomainfrom
dta/fix_for_sumo
Open

Changes for Sumo#116
bhung-bdai wants to merge 12 commits intomainfrom
dta/fix_for_sumo

Conversation

@bhung-bdai
Copy link
Copy Markdown
Collaborator

Alterations which enable support for Sumo and other C++ backend rollouts.

@bhung-bdai bhung-bdai marked this pull request as ready for review March 24, 2026 19:11
Copy link
Copy Markdown
Collaborator

@slecleach slecleach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the Sumo backend work — I walked through the diff in detail and found two behavioral regressions to fix before merge:

  1. SimulationNode._init_sim can now fail for cfg-registered custom tasks
  • In judo/app/dora/simulation.py, _init_sim() calls get_registered_tasks().get(task_name) before any backend instance is constructed.
  • Custom tasks are registered in Simulation.__init__ via register_tasks_from_cfg(task_registration_cfg), which only runs when sim_backend_cls(...) is instantiated.
  • That means init_task values supplied only through task_registration_cfg now raise ValueError during node startup.
  1. Switching task resets controller state/config unexpectedly
  • In judo/app/dora/controller.py, update_task() now rebuilds the controller via self._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 fresh ControllerConfig.
  • 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.

Comment thread judo/simulation/base.py Outdated
Comment thread judo/simulation/mj_simulation.py
Comment thread judo/simulation/mj_simulation.py
@bhung-bdai
Copy link
Copy Markdown
Collaborator Author

  1. SimulationNode._init_sim

I see the concern in point 1, but we never supply the init_task in the task_registration_cfg by itself (especially during a task switch).

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 run_mpc in the future but this will be fixed by registering the config first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants