reproducible keyed domain-randomization sampling#146
Draft
juan-g-bonilla wants to merge 7 commits into
Draft
Conversation
juan-g-bonilla
changed the title
Replace the negative `-m "not isaacsim"` style of test deselection with a positive, mutually-exclusive run-target marker taxonomy. `conftest.py` now auto-marks every collected test with exactly one backend target (isaacgym/isaacsim/mujoco/no_sim/requires_inference): tests under tests/simulators/<backend>/ inherit that backend, mujoco_classic/mujoco_warp imply mujoco, and everything else falls back to no_sim. The eight markers are registered in pyproject.toml so `--strict-markers` can be turned on. Motivation: negative selection silently skips a test whenever a new backend or directory is added — the test simply matches no exclusion and never runs. Positive selection makes "which lane runs this test" explicit and fails loud on an unregistered marker. Adds the CPU-only no_sim lane and a mujoco lane, switches the existing isaacgym/isaacsim CI scripts to positive selection, and gives the e2e / push-randomization tests (which live outside tests/simulators/) their explicit isaacgym marker. test_torch_utils pins device="cpu" so it passes in the GPU-less no_sim job.
Build the MuJoCo-Warp GPU image in CI and locally. setup_mujoco.sh gains a `--skip-driver-check` flag (and SKIP_DRIVER_CHECK env var) that gates the nvidia-smi probe, so the Warp install can run inside a `docker build` on a GPU-less builder where no driver is visible. The mujoco Dockerfile's WARP=true branch passes that flag; build_and_push.sh and the docker-build workflow thread a per-image `WARP=true` build-arg through to `docker build`. Motivation: the Warp image previously could not build in CI because the build host has no GPU, so the driver check aborted the install. Decoupling the build-time install from the runtime driver check lets the image build on any runner while still checking the driver at run time.
Behavior-preserving cleanup surfaced while preparing the feature work: - warp_utils.py reformatted to 4-space indentation with a NOTE that the float()/int() literal wrappers are load-bearing for NVRTC (a bare literal infers as const and cannot bind to a Warp mutable out-param); ruff UP018/RUF046 are disabled for this file in pyproject.toml so an autofix cannot strip them. - setup.py renames the `min` loop variable (shadowed the builtin); the mypy pre-commit hook now excludes any src/*/setup.py packaging shim. - Import-order / formatting only: rotations.py, tyro_utils.py, reset_events/manager.py, terrain/base.py, config_types/observation.py. - Dead-code / suppression removal: config_utils.py (unused imports), run_sim.py (stale noqa), mujoco/backends/__init__.py (collapse the unused TYPE_CHECKING WarpBackend alias into the single import guard). - terrain/terms/locomotion.py pins torch.meshgrid(..., indexing="ij") to silence the PyTorch deprecation warning. - test_torch_jit.py: comment-only clarification. Motivation: keep these mechanical changes out of the feature diffs so reviewers see no behavior change here.
UndesiredContacts (reward) and PenaltyCurriculum (curriculum) accept a regex / tag that selects the bodies they act on. A pattern that matched nothing produced a silently inert term: an "enabled" penalty that is a permanent no-op, or a curriculum whose tag never lines up with an active reward term. Both now log a warning naming the unmatched pattern. Motivation: a misspelled body pattern or reward tag is a common config mistake that otherwise disappears without a trace and is only noticed as a training regression. The reward term also hoists its pattern/body-name locals to drop two type: ignore comments along the way.
Add resolve_asset_path() for turning a config asset string into a concrete path,
and generalize the package-path resolution it shares with resolve_data_file_path.
The package-path trigger widens from the `holosoma/data` prefix (resolved against
files("holosoma.data")) to the `holosoma/` prefix (resolved against
files("holosoma")), and an `@holosoma/...` spelling alias is accepted for both.
Self-locating inputs (holosoma/, @holosoma/, s3://, absolute paths) pass through;
other relative inputs resolve against the package data root.
Motivation: the scene-asset spawners (added in following commits) need one
backend-agnostic way to resolve an asset path regardless of the working directory
or how the package was installed. Widening the prefix to `holosoma/` lets assets
outside data/ (and the @-alias form used by the new scene configs) resolve through
the same code. Note this also changes resolve_data_file_path's accepted prefix for
existing callers — they continue to work because `holosoma/data/...` still matches
the broader `holosoma/` prefix. Covered by test_path_resolution.py.
…nd randomize_field Introduce the single source of truth for domain-randomization sampling: - config_types/distribution.py (import-light, no torch): DistributionSpec and the DistributionLike config union, with a bounds-first convention so `[lo, hi]` means the same thing for uniform/log_uniform/gaussian on every backend. Validation runs once at construction and fails loud instead of producing a silent NaN deep in a physics write. - utils/sampler.py: the keyed, counter-based TermSampler. A draw is value = f(base_seed, term, stage, env, episode, coords) via a SplitMix64 fold, so the same (term, env, episode) draws the same value regardless of term set, term order, num_envs, or which env subset is resetting; the vectorized path is bit-identical to a per-env loop. Includes a shared inverse-CDF (uniform / log_uniform / true truncated-normal, one-sided supported) and quantiles()/ permute() for reproducing a continuous spec through PhysX's material-bucket cap. Also renames simulator/mujoco/backends/warp_randomization.py to randomization.py and makes randomize_field drive both MuJoCo backends (Warp GPU per-world bridge and Classic CPU in-place MjModel field write), updating the fields.py import. Motivation: DR was previously sampled ad hoc per term/backend, with `[lo, hi]` interpreted inconsistently and no reproducibility guarantee, and randomize_field assumed the Warp backend. This is the chokepoint every later DR term routes through. Covered by test_distributions.py and test_keyed_distributions.py (both CPU-only).
Wire the DistributionSpec/TermSampler introduced by the previous commit into the randomization pipeline so every robot-DR draw is reproducible per (term, env, episode): - RandomizationManager binds a TermSampler (keyed on the term name, lifecycle stage, and per-env episode counter) and passes it into each term's setup/reset/step. - RandomizationTermBase.setup/reset/step gain the sampler parameter; robot-DR terms (base_com / mass / friction / pd_gains / push / actuator / dof-bias / rfi) draw through it instead of raw generators. - BaseTask gains dr_base_seed + dr_episode_count (the keying state the manager binds against; the per-env counter advances before reset-stage DR). - config_types.randomization gains BaseComRange (typed per-axis CoM range for randomize_base_com_startup). New terms/_shared.py holds the small cross-term helpers (_ensure_env_ids_tensor, the IsaacSim material bucket count). The object physics DR terms (randomize_object_rigid_body_*) stay in locomotion.py in their existing IsaacSim-only form here; the scene-objects feature branch relocates them to terms/objects.py and makes them cross-backend + registry-driven. Self-contained and mergeable: mypy + no_sim green; robot-DR is fully sampler-driven.
juan-g-bonilla
force-pushed
the
dev/gjuaga/02_reproducible_dr_sampling
branch
from
85316f9 to
8164f53
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Base:
dev/gjuaga/01_prep_ci_and_utilsSummary
Make domain randomization reproducible and order-independent. Introduce a single validated
DistributionSpec(bounds-first, checked once) and a keyed counter-basedTermSampler, so thesame
(term, env, episode)draws the same value regardless of the term set/order,num_envs, orwhich env subset resets, and thread that sampler through the randomization manager and the
robot-DR terms.
What's in it
feat(dr):DistributionSpec(config_types/distribution.py) + keyedTermSampler(
utils/sampler.py); renamewarp_randomization.py→randomization.pywith arandomize_fieldthat draws through the sampler and drives both MuJoCo backends.feat(dr)(threading):RandomizationManagerbinds aTermSamplerper (term, stage,episode) and passes it into each term;
RandomizationTermBase.setup/reset/stepgain thesamplerparameter; robot-DR terms (base_com/mass/friction/pd_gains/push/actuator/…)draw through it.
BaseTaskgainsdr_base_seed+dr_episode_count(the keying state);config_types.randomization.BaseComRange; sharedterms/_shared.pyhelpers.Risk
Low, non-breaking for config authors. Custom
RandomizationTermBasesubclasses / direct DR termcalls now take a keyword-only
sampler: TermSampler(see migration guide §2.3), but config-drivenDR is unaffected (the manager injects the sampler). Reproducibility of DR draws changes (now keyed),
which is the point of the feature.
Migration Guide
1 Custom
RandomizationTermBasesubclasses / direct term calls gainsamplersetup/reset/stepand all term functions now require a keyword-onlysampler: TermSampler. Bind withTermSampler.bind(seed, term_name, stage, episode_count).Config-only users need no change — the
RandomizationManagerinjects thesamplerfor you.Range params now accept the same
[lo, hi]form but are typedDistributionLike(also accepting{kind, low, high, mean, std}for gaussian / log_uniform; bounds must be finite, log_uniformbounds strictly positive — validated at construction).
base_com_rangeis typedBaseComRange.2 Module rename
simulator.mujoco.backends.warp_randomizationsimulator.mujoco.backends.randomization