[eval callback]Refactor eval callbacks into grid-based multi-condition sweep architecture by Juyue · Pull Request #101 · amazon-far/holosoma

Juyue · 2026-04-20T19:02:03Z

Replace single-env push/payload callbacks with grid evaluation system that sweeps across multiple conditions in parallel. Velocity is always the base axis; either payload or push can be cross-producted with it (but not both, since they share the IsaacSim external force API).

Key changes:

GridConditionManager: unified add_axis() API for Cartesian product of sweep axes, shared across all callbacks
EvalRecordingCallback: single recorder with ensure_finalized() pattern for order-independent deferred setup
GridEvalVelocityCallback: independent velocity sweep (lin_x/y, ang_yaw)
GridEvalPayloadCallback: body-group x mass sweep with per-substep force injection via monkey-patched physics step
GridEvalPushCallback: body x direction x gait_phase x force sweep with GaitAnalyser for phase-triggered pushes
GaitAnalyser: standalone gait cycle detection from foot heights
sim_utils: resolve_body(), apply_body_force_world(), clear_body_force()
Config validation via @model_validator (body/label lengths, directions, gait phases, mutual exclusion of payload+push)

Minor:

Update src/holosoma/holosoma/config_values/loco/g1/reward.py to reduce visual noise.

samuelgundry

Part-way through, up to src/holosoma/holosoma/agents/callbacks/grid_eval_payload.py

samuelgundry · 2026-04-21T11:35:47Z

+    def _require_recording_cb(self) -> Any:
+        """Find and return the EvalRecordingCallback, or raise."""
+        from holosoma.agents.callbacks.recording import EvalRecordingCallback
+
+        for cb in self.training_loop.eval_callbacks:
+            if isinstance(cb, EvalRecordingCallback):
+                return cb
+        raise RuntimeError(f"{type(self).__name__} requires EvalRecordingCallback. Set --recording.config.enabled=True")


Will there ever be a second eval CB? defensively, it can help to check here and error if so.

Theoretically, no second EvalRecordingCallback shows up.

Added check to be denfensive.

samuelgundry · 2026-04-21T11:37:15Z

+
+        self._left_foot_z_range = (np.min(left_z, axis=0), np.max(left_z, axis=0))
+        self._right_foot_z_range = (np.min(right_z, axis=0), np.max(right_z, axis=0))
+        self._done = True


perhaps set done at the end of the function, just in case of confusion w.r.t debugging

Thanks for suggestions. Done

samuelgundry · 2026-04-21T11:40:06Z

+    def add_axis(
+        self,
+        name: str | list[str],
+        values: list,


what happens for duplicate axis names? "add" vs. "register". Good to document the behavior/expectations. You append below without checking but idk (yet) if you expect a single name.

edit: i see below you de-dupe. Good to document then, especially for callers -- right now, there's no way to know a duplication happened and the match is by key only? not values

Register seems more appropriate.

Add check for duplicated axes name.

Move the dedup code to where it actually belong (src/holosoma/holosoma/agents/callbacks/grid_eval_velocity.py)

samuelgundry · 2026-04-21T11:42:50Z

+        unique: list[dict[str, Any]] = []
+        for cond in self.conditions:
+            key = tuple(sorted(cond.items()))
+            if key not in seen:


I suspect at the least you'll want to log duplicates, but see above re: comparing keys vs. values, and what callers can/can't do if there are duplicates with different values

Good call!

Now, I move the dedup code from grid_conditions to where it actually belong (src/holosoma/holosoma/agents/callbacks/grid_eval_velocity.py).

Grid_conditions now just does cartesian product without .

samuelgundry · 2026-04-21T11:43:40Z

+    def get_metadata(self) -> dict[str, Any]:
+        """Return condition metadata for NPZ recording.
+
+        ``grid_conditions`` uses hierarchical dicts grouped by the ``group``
+        parameter passed to ``add_axis()``.  Ungrouped keys stay at
+        the top level.
+
+        Example condition::
+
+            {'velocity': {'lin_vel_x': 0.5, 'lin_vel_y': 0.0, 'ang_vel_yaw': 0.0},
+             'push': {'body_label': 'torso', 'direction': 'forward', 'force_n': 150.0}}
+        """


I don't know our current position towards unit testing, but this kind of logic is a good reason to add them, especially for future regressions.

Yes, this part is a bit messy.

To simplify the code, I removed some unused/convenience meta data.

Instead of having a unit test, I have a doc-string to demonstrate the intended behavior.

Add shared infrastructure for grid-based multi-condition eval sweeps: - base_callback: _get_env() and _require_recording_cb() helpers - grid_conditions: GridConditionManager for Cartesian-product sweep axes - sim_utils: body resolution and force application helpers - gait_analysis: GaitAnalyser for foot-height-based phase detection Also pin pydantic<2.11 in pre-commit config to fix mypy hook (pydantic 2.11+ uses parenthesized context managers incompatible with mypy's python_version=3.8 parsing). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Replace single-env push/payload callbacks with grid-based multi-condition sweep architecture. Recording callback now records all condition envs with shape [T, num_conditions, ...]. - grid_eval_velocity: per-env velocity command injection with warmup - grid_eval_push: deterministic push sweeps (body x dir x gait_phase x force) - grid_eval_payload: payload sweeps (body_group x mass) - recording: grid-aware recorder with condition_manager, buffer API - eval_callback configs: GridEvalVelocityConfig, GridEvalPushConfig, GridEvalPayloadConfig replace old PushConfig/PayloadConfig - Delete old push.py and payload.py

- Extract shared reward terms into _g1_29dof_loco_common_terms dict (used by both PPO and FastSAC presets, only 'alive' weight differs) - Add DEBUG_ZERO_STEP / DEBUG_FREEZE_FRAME env vars for WBT debugging (freezes motion advance at a specific frame when enabled)

…rding cb - EvalCallbacksConfig._validate: raise if any grid callback is enabled without recording.config.enabled - _require_recording_cb: collect all matches, raise on 0 or >1

…ze() GridConditionManager.finalize() no longer silently deduplicates conditions. Instead, GridEvalVelocityCallback deduplicates its own values before calling add_axis (the only source of duplicates was multiple velocity axes including zero). The manager's contract is now: axes in, cross-product out, no magic.

…labels, simplify metadata - Rename add_axis() to register_axis() across all callers - Remove unused labels parameter (no downstream consumer) - Add duplicate axis key detection in register_axis() - Simplify get_metadata(): remove sweep_*_values section, add docstring example - Move _done assignment in GaitAnalyser to after logging (cosmetic) - Remove stale sweep_*_values line from README

Juyue requested a review from samuelgundry April 20, 2026 19:02

samuelgundry reviewed Apr 21, 2026

View reviewed changes

Juyue and others added 8 commits July 20, 2026 17:15

remove DEBUG_ZERO_STEP / DEBUG_FREEZE_FRAME from wbt.py

6886376

docs: add README for eval callbacks system

bcce6ea

validate recording requirement at config level + catch duplicate reco…

ba67c2e

…rding cb - EvalCallbacksConfig._validate: raise if any grid callback is enabled without recording.config.enabled - _require_recording_cb: collect all matches, raise on 0 or >1

Juyue force-pushed the dev/jchen/grid_eval_callbacks branch from 20029c8 to d144d92 Compare July 21, 2026 18:17

Drop pydantic<2.11 pin from pre-commit config

4901f5a

Navigation Menu

[eval callback]Refactor eval callbacks into grid-based multi-condition sweep architecture - #101

[eval callback]Refactor eval callbacks into grid-based multi-condition sweep architecture#101
Juyue wants to merge 9 commits into
mainfrom
dev/jchen/grid_eval_callbacks

Juyue commented Apr 20, 2026

Uh oh!

samuelgundry left a comment

Uh oh!

samuelgundry Apr 21, 2026

Uh oh!

Juyue Jul 21, 2026

Uh oh!

samuelgundry Apr 21, 2026

Uh oh!

Juyue Jul 21, 2026

Uh oh!

samuelgundry Apr 21, 2026

Uh oh!

Juyue Jul 21, 2026

Uh oh!

samuelgundry Apr 21, 2026

Uh oh!

Juyue Jul 21, 2026 •

edited

Loading

Uh oh!

samuelgundry Apr 21, 2026

Uh oh!

Juyue Jul 21, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Juyue commented Apr 20, 2026

Uh oh!

samuelgundry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juyue Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Juyue Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Juyue Jul 21, 2026 •

edited

Loading

Juyue Jul 21, 2026 •

edited

Loading