ci(gpu): point LIBERO at bundled package assets in nightly test#322
Conversation
test_control_freq_reaches_real_robosuite_sim builds a real OffScreenRenderEnv, which needs LIBERO's bddl/init/asset files. The workflow set LIBERO_CONFIG_PATH to an empty /tmp tree (correct for the mocked CPU tests, which patch get_libero_path), so the test failed with FileNotFoundError on a .pruned_init file. Generate the config from the assets bundled in the installed package (the fork's MANIFEST grafts bddl_files/init_files/assets) via set_libero_default_path(). Also set init_states=False in the test to focus it on the control_freq assertion; the env build still exercises the real bddl/asset path, so init_states alone would not have fixed it.
shuheng-liu
left a comment
There was a problem hiding this comment.
Review: the LIBERO-assets CI fix is correct and empirically validated.
I traced the workflow incantation end-to-end against the pinned LIBERO fork (45c5a01) and the mechanics hold:
touch "$LIBERO_CONFIG_PATH/config.yaml"—libero/libero/__init__.pyonly prompts onif not os.path.exists(config_file). Pre-creating the file skips the interactive first-runinput(), which would otherwise hit EOF and crash the import under CI. The empty file is harmless because nothing reads the config at import time (get_libero_pathis lazy).set_libero_default_path()— readsLIBERO_CONFIG_PATHfrom the env and writes package-relative paths (benchmark_root= the installedlibero/libero/dir) into that sameconfig.yaml, overwriting the empty file. The subsequentpytestinherits the exportedLIBERO_CONFIG_PATH, soget_libero_path("bddl_files")resolves into the installed package.- The assets actually land in the install:
MANIFEST.indoesgraft libero, and — the load-bearing part —setup.pysetsinclude_package_data=True, so the graftedbddl_files/init_files/assetsend up in site-packages, not just the sdist.
Blast radius: the env-var change applies to the whole pytest -m "gpu" run, but test_control_freq_reaches_real_robosuite_sim is the only GPU-marked test that touches LIBERO config paths — every other LIBERO test is CPU/mocked and patches get_libero_path. No collateral on the other GPU policy tests.
init_states=False: doesn't weaken the test — the control_freq read-back is independent of init states, it matches the two mocked siblings, and the real bddl path is still exercised by _make_envs_task. Belt-and-suspenders with the workflow fix, which is fine.
SC2155: accurate — switching export VAR="$(pwd)/..." to a literal removes the command-substitution-masks-return-value warning, and the new quoted mkdir/touch lines don't introduce any.
Validation: the manually-dispatched GPU run (26306227758) shows Run Pytest on GPU = success; CPU / pre-commit / check-checklist are all green.
Two optional, non-blocking notes:
- The workflows now use two different LIBERO config strategies — gpu points at the installed package via
set_libero_default_path(), while cpu/regression still point at the empty.github/assets/liberotree. Intentional and explained in the inline comment, but a future reader editing one might not notice the other differs; a one-line cross-pointer would help. set_libero_default_path()trusts that the install contains the assets. If a future LIBERO bump dropped a file (or moved assets to LFS without materializing them), this would resurface as aFileNotFoundErrorat env-build time rather than anything clearer. Not worth guarding now — the nightly is the right place to catch it.
LGTM.
Generated by Claude Code
What this does
Fixes #319.
The nightly GPU test
test_control_freq_reaches_real_robosuite_sim(added in #312) builds a realOffScreenRenderEnvto verify the configuredcontrol_freqactually reaches robosuite — the mocked sibling tests can't catch robosuite silently dropping the kwarg. Building a real env needs LIBERO'sbddl_files/init_files/assets, butgpu_test.ymlsetLIBERO_CONFIG_PATHto an empty/tmptree, so the test failed with:The empty-tree config is correct for the mocked CPU tests (they patch
get_libero_path), but the real-sim GPU test needs actual assets. Note that just settinginit_states=Falsein the test is not sufficient on its own: the env build insrc/opentau/envs/libero.pyreads the bddl file viaget_libero_path("bddl_files"), so it would then fail on the missing bddl file instead. The workflow fix is the necessary part.Changes:
.github/workflows/gpu_test.yml: pointLIBERO_CONFIG_PATHat the assets bundled inside the installed LIBERO package (the fork'sMANIFEST.indoesgraft libero, shippingbddl_files/init_files/assets) via LIBERO's ownset_libero_default_path(), instead of the empty/tmptree. As a bonus this also drops a pre-existingSC2155shellcheck warning on the oldexport VAR="$(pwd)/..."line.tests/envs/test_libero_control_freq.py: setinit_states=Falseto focus the test on itscontrol_freqassertion (matching the two mocked sibling tests); the env build still exercises the real bddl/asset path.How it was tested
actionlinton the edited workflow: theRun Testsstep is shellcheck-clean, and the change removes a pre-existingSC2155warning.init_files/libero_10/*.pruned_init(~23 KB real file, not an LFS pointer) and thebddl_files/libero_10/*.bddlfor the task under test, and thatMANIFEST.ingrafts them into the installed package.gpu_test.ymlwas manually dispatched on this branch viaworkflow_dispatch; see the linked run in the checks.How to checkout & try? (for the reviewer)
# Dispatch the nightly GPU workflow on this branch: gh workflow run gpu_test.yml --ref claude/magical-banach-eb58a8# On a CUDA machine with the libero extra installed, run just the fixed test: pytest -sx tests/envs/test_libero_control_freq.py::test_control_freq_reaches_real_robosuite_simChecklist