feat(genesis): add Genesis physics simulation environment family#32
feat(genesis): add Genesis physics simulation environment family#32
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
kinitro/environments/registry.py (1)
120-131:⚠️ Potential issue | 🟡 MinorStale docstring: "(currently only 'metaworld')".
Line 126 says
family: Environment family (currently only 'metaworld')— this was already outdated with ProcTHOR and is now further outdated with Genesis. Update to reflect all supported families or remove the parenthetical.Proposed fix
- family: Environment family (currently only 'metaworld') + family: Environment family (e.g., 'metaworld', 'procthor', 'genesis')
🤖 Fix all issues with AI agents
In `@environments/genesis/Dockerfile`:
- Around line 21-76: Add a non-root user in the Dockerfile, create a dedicated
user/group (e.g., "appuser"), chown the runtime directories (/app and
/opt/menagerie) and any files created at build time, and then switch to that
user with a USER directive before the final image is emitted; specifically:
after installing system deps and copying files (after pip install and the git
clone), add commands to create the user/group (useradd or addgroup/adduser), run
chown -R appuser:appuser /app /opt/menagerie (and adjust ownership of any other
runtime paths), and add USER appuser so the container runs non-root while
keeping existing ENV/PYTHONPATH settings intact.
In `@environments/genesis/env.py`:
- Around line 294-310: The returned metadata currently includes the miner's
base_url inside the "extra" dict (key "base_url"), which may leak sensitive
deployment tokens; remove the "base_url" entry (or replace it with a
redacted/boolean flag) from the dict returned by the function that constructs
this result (the block building the "extra" dict including task_id, seed,
env_id, timesteps, total_reward, mean_action_time, max_action_time, model,
base_url) so logs no longer contain the raw URL, and update any callers/tests
that expect base_url accordingly.
- Around line 134-148: The base_url None check currently raises a raw ValueError
before the surrounding try/except and should instead return a structured error
like the env_id check; modify the genesis environment handler so that when
base_url is None it calls self._build_error_result(...) with the same keys
(env_id, task_id, seed or task_id, start_time=time.time(), and an explanatory
error message) or move the base_url validation inside the existing try block so
that any missing-base_url condition is caught and converted into the structured
error result; update the check near env_id/seed handling that references env_id,
base_url, seed, task_id and uses _build_error_result to ensure consistent error
behavior.
In `@kinitro/cli/env/commands.py`:
- Line 24: The build_env docstring for the function build_env should be updated
to mention the new "genesis" family under the "Environment families" section;
add a short description for Genesis (e.g., "Genesis — custom benchmark
environments") and an approximate image size similar to the other entries (e.g.,
"~1.2GB") so the help text matches AVAILABLE_ENV_FAMILIES which now includes
"genesis".
In `@kinitro/environments/genesis/base.py`:
- Around line 534-548: The code slices joint_action to n but doesn't handle when
joint_action has fewer than n elements, causing a NumPy shape mismatch at
target_pos = default_pos + joint_action * action_scale; modify the logic around
joint_action (after obtaining joint_action and computing n) to check its length
and if shorter than n pad it with zeros (as float32) to length n before clipping
and mapping; ensure the padded joint_action matches the dtype and shape of
default_pos and action_scale so target_pos computation (using joint_action,
default_pos, action_scale) succeeds without errors.
In `@kinitro/environments/genesis/scene_generator.py`:
- Around line 219-239: The stepped-terrain height math in _get_terrain_height
contradicts the nested-box geometry created in build_scene: _get_terrain_height
currently increases height with radial distance while build_scene stacks boxes
of increasing size (and height) from center outward, causing center points to be
under the tallest outer box. Fix by making the two match: either modify
build_scene to invert the step stacking so inner boxes are taller (so height
decreases with radius), or update _get_terrain_height to compute heights that
match the nested-box geometry (e.g., derive step index from box size ordering
used in build_scene or compute height as total_steps - step_idx times
step_height). Update the logic in the functions _get_terrain_height and/or
build_scene accordingly and ensure step_width/step_height params remain
consistent between them.
- Around line 248-251: The CI linter flags the runtime-only import "import
genesis as gs" as unresolved; fix it by updating that import to suppress the
type checker—change the line "import genesis as gs # noqa: PLC0415" to include
a mypy ignore like "import genesis as gs # noqa: PLC0415 # type:
ignore[import-unresolved]" (or alternatively implement a conditional import
guard or small local "genesis" stub) so the linter stops reporting the
unresolved import while keeping the runtime behavior intact.
- Around line 186-217: The fallback in _generate_object_position currently
returns a hardcoded [1.5, 0.0, 0.05] which can place objects below the surface
on non-flat terrain; change the fallback to compute terrain height using
_get_terrain_height(1.5, 0.0, terrain_type, terrain_params) and add the same
small offset (0.05) before returning so the final fallback is [1.5, 0.0,
terrain_height + 0.05]; keep the existing signature and semantics of
_generate_object_position unchanged.
In `@kinitro/environments/genesis/task_generator.py`:
- Around line 120-131: The current loop silently overrides an explicitly passed
task_type on retries; change the logic in the method containing the loop so that
if the caller provided a non-None task_type you do not replace it on subsequent
attempts—only call _random_choice(available_types, rng) when the initial
task_type was None. Concretely: capture a flag like "explicit_type_provided =
(task_type is not None)" before the loop, call self._try_generate_task(objects,
rng, task_type, robot_config) up to self._max_attempts keeping the same
task_type when explicit_type_provided is True, and only re-select task_type from
available_types via _random_choice when explicit_type_provided is False; ensure
the function returns None after exhausting attempts. Reference symbols:
task_type, available_types, _random_choice, _try_generate_task,
self._max_attempts.
In `@kinitro/environments/genesis/task_types.py`:
- Around line 104-146: check_task_feasibility currently verifies destination for
TaskType.PLACE but doesn't guard against target == destination; update the
TaskType.PLACE branch in check_task_feasibility to mirror the PUSH checks by
returning infeasible when destination is None or when target.object_id ==
destination.object_id (include a clear reason string like "Cannot place object
at its own location"), referencing the TaskType.PLACE branch and the
target/destination object_id comparisons to locate the change.
🧹 Nitpick comments (16)
environments/genesis/requirements.txt (1)
1-8: Consider pinning dependency versions for reproducible Docker builds.Most packages here are unpinned. In particular,
torchandgenesis-worldare fast-moving libraries where minor version bumps can introduce breaking changes or significantly change behavior. Since this file is used in a Dockerfile for evaluation, reproducibility matters. At minimum, pingenesis-worldandtorchto known-good versions.environments/genesis/Dockerfile (1)
56-62: Pin the MuJoCo Menagerie clone to a specific commit or tag.
git clone --depth 1fetches whatever is atHEAD, so builds are non-reproducible and an upstream breaking change to the G1 MJCF files could silently break the image. Pin to a known-good commit hash or tagged release.Example with a pinned commit
-RUN git clone --depth 1 --filter=blob:none --sparse \ - https://github.com/google-deepmind/mujoco_menagerie.git /opt/menagerie && \ - cd /opt/menagerie && \ - git sparse-checkout set unitree_g1 && \ - echo "Menagerie assets installed" +ARG MENAGERIE_COMMIT=<pinned_commit_hash> +RUN git clone --filter=blob:none --sparse \ + https://github.com/google-deepmind/mujoco_menagerie.git /opt/menagerie && \ + cd /opt/menagerie && \ + git checkout ${MENAGERIE_COMMIT} && \ + git sparse-checkout set unitree_g1 && \ + echo "Menagerie assets installed at ${MENAGERIE_COMMIT}"kinitro/environments/registry.py (1)
48-58: LGTM — consider a safeguard ongetattr.The dynamic
getattr(envs, env_cls_name)approach is more flexible than dedicated per-class imports and scales well for adding more Genesis variants. The trade-off is that a typo inenv_cls_namesurfaces as anAttributeErroronly at first instantiation time rather than at import time. Since the string is a literal at the call site, this is acceptable, but you might want to add a brief guard for a clearer error message:Optional: friendlier error on missing class
cls = getattr(envs, env_cls_name) + if cls is None: + raise ImportError(f"Genesis environment class '{env_cls_name}' not found in envs module")Or use
getattr(envs, env_cls_name, None)with a check.kinitro/environments/genesis/task_generator.py (2)
53-58:_random_choiceusesTypeVar("T")but consider simplifying with a generic.The
TypeVar("T")at line 18 is only used here. This is fine, though in Python 3.12+ you could use the[T]syntax on the function. Minor nit — no action needed now.
66-68: Remove unused helper function_get_landmark_objectsor document its intended purpose.This function is defined but never called anywhere in the codebase. Per YAGNI, remove it unless there's a documented future use case.
kinitro/environments/genesis/envs/g1_humanoid.py (1)
35-81:target_poscomputed on line 48 is unused for non-NAVIGATE branches.Lines 47–48 unconditionally compute
robot_posandtarget_pos, buttarget_posis only used inside theNAVIGATEbranch (line 52). For PICKUP, PLACE, and PUSH the method usesobj_posanddest_posinstead. This is harmless (cheap computation), but pullingtarget_posinto the NAVIGATE block would improve readability.kinitro/environments/genesis/robot_config.py (1)
8-36: Consider usinglist[TaskType]instead oflist[str]forsupported_task_types.
supported_task_typesstores string values (e.g.,"navigate") that must matchTaskType.value. This stringly-typed coupling is visible intask_generator.pyline 111 (t.value in robot_config.supported_task_types). Usinglist[TaskType]directly would provide type safety and eliminate the risk of drift if enum values change.That said, keeping it as strings may be intentional for serialization or decoupling — so flagging as optional.
kinitro/environments/genesis/scene_generator.py (1)
17-17:loggeris defined but never used in this module.environments/genesis/env.py (2)
60-85: Newhttpx.AsyncClientper call creates connection overhead on the hot path.Each of the up to 500
/actcalls creates a newAsyncClient, establishing a fresh connection pool. The comment explains this is to avoid event loop binding issues with affinetes, which is a valid concern.Consider creating the client once at the start of
_run_evaluationand reusing it for all calls within that evaluation, then closing it at the end. This avoids both the per-call overhead and the cross-event-loop issue.
174-220: Miner reset is called before env reset — consider ordering implications.Line 204 calls the miner's
/resetendpoint, then line 220 callsenv.reset(task_config). Ifenv.reset()fails (e.g., Genesis engine crash), the miner has already been reset and is waiting for observations that will never arrive. Consider reversing the order: reset the simulation first (which is more likely to fail), then notify the miner.kinitro/environments/genesis/base.py (6)
33-38: Log the GPU initialization failure before falling back to CPU.When GPU init fails, the exception is silently swallowed. Logging it at
debugorwarninglevel would help troubleshoot GPU driver/availability issues in production.🔧 Proposed fix
try: gs.init(backend=getattr(gs, "gpu")) logger.info("genesis_initialized", backend="gpu") - except Exception: + except Exception as e: + logger.debug("genesis_gpu_init_failed", error=str(e)) gs.init(backend=getattr(gs, "cpu")) logger.info("genesis_initialized", backend="cpu")
77-80: Add brief comments explaining theAnytype hints.Per coding guidelines: "Avoid
Anytype hints unless required; if used, explain why in code comments." These areAnybecause thegenesismodule is only available at runtime inside Docker, so the concrete types (gs.Scene,gs.Entity,gs.Camera) can't be referenced at import/lint time.📝 Proposed fix
# Lazy-initialized components - self._scene: Any = None - self._robot: Any = None - self._camera: Any = None - self._object_entities: list[Any] = [] + # Genesis types (gs.Scene, gs.Entity, gs.Camera) are unavailable at + # import time — the engine is a Docker-only runtime dependency. + self._scene: Any = None # gs.Scene + self._robot: Any = None # gs.Entity (MJCF morph) + self._camera: Any = None # gs.Camera + self._object_entities: list[Any] = [] # list[gs.Entity]As per coding guidelines, "Avoid
Anytype hints unless required; if used, explain why in code comments."
222-226: Redundant local re-import ofSceneConfig.
SceneConfigis already imported at the module level (line 14). OnlySceneObjectConfigneeds a local import here.📝 Proposed fix
- from kinitro.environments.genesis.scene_generator import ( # noqa: PLC0415 - SceneConfig, - SceneObjectConfig, - ) + from kinitro.environments.genesis.scene_generator import SceneObjectConfig # noqa: PLC0415
278-296: Consider the success + fallen interaction in the same step.If
_check_successreturnsTruebut the robot has also fallen,_episode_successis latched toTruewhile the reward is overwritten to-1.0. Downstream,get_success()will report success despite a penalty. If this is intentional (task completion is "sticky" regardless of final posture), a brief comment would clarify the design choice for future maintainers.
379-383: Extract the hardcoded floating-base DOF count (6) into a named constant.The magic number
6(floating-base DOFs) appears in four places: here,_read_robot_state(lines 445–446), and_apply_action(line 551). If a fixed-base robot is ever added, all locations must be updated in lockstep. A class constant (or aRobotConfigfield) would make this explicit and prevent drift.♻️ Proposed fix
Add a class-level constant:
# Rendering settings IMAGE_SIZE = 84 SIM_DT = 0.01 # 100 Hz physics CONTROL_DT = 0.02 # 50 Hz control (2 physics steps per control step) + FLOATING_BASE_DOFS = 6 # 3 translation + 3 rotation for free-floating robotsThen replace all occurrences, e.g.:
- actuated_dof_idx = list(range(6, 6 + n)) + actuated_dof_idx = list(range(self.FLOATING_BASE_DOFS, self.FLOATING_BASE_DOFS + n))
463-467: Silent exception swallowing hides object-state read failures.The bare
except Exception: passmakes it impossible to diagnose issues where object positions can't be read (e.g., entity destroyed, tensor device mismatch). At minimum, log a warning so failures are visible.🔧 Proposed fix
try: pos = entity.get_pos().cpu().numpy().flatten() states[obj_id] = pos - except Exception: - pass + except Exception as e: + logger.debug("object_state_read_failed", object_id=obj_id, error=str(e))
| FROM python:3.11-slim | ||
|
|
||
| WORKDIR /app | ||
|
|
||
| # Install system dependencies for Genesis rendering (EGL/OpenGL headless) | ||
| RUN apt-get update && apt-get install -y --no-install-recommends \ | ||
| git \ | ||
| # OpenGL/Mesa libraries for headless rendering | ||
| libgl1 \ | ||
| libgl1-mesa-dri \ | ||
| libglib2.0-0 \ | ||
| libsm6 \ | ||
| libxext6 \ | ||
| libxrender1 \ | ||
| libgomp1 \ | ||
| libegl1 \ | ||
| libegl-mesa0 \ | ||
| libglvnd0 \ | ||
| libglx0 \ | ||
| libglx-mesa0 \ | ||
| mesa-utils \ | ||
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # EGL for headless rendering | ||
| ENV PYOPENGL_PLATFORM=egl | ||
| ENV NVIDIA_DRIVER_CAPABILITIES=all | ||
|
|
||
| # Force software rendering when no GPU available | ||
| ENV LIBGL_ALWAYS_SOFTWARE=1 | ||
| ENV MESA_GL_VERSION_OVERRIDE=3.3 | ||
|
|
||
| # Copy requirements and install dependencies | ||
| COPY requirements.txt /app/ | ||
| RUN pip install --no-cache-dir -r /app/requirements.txt | ||
|
|
||
| # Clone MuJoCo Menagerie (sparse checkout -- only robot dirs needed) | ||
| # This provides MJCF assets for supported robots (G1, Go2, Franka, etc.) | ||
| RUN git clone --depth 1 --filter=blob:none --sparse \ | ||
| https://github.com/google-deepmind/mujoco_menagerie.git /opt/menagerie && \ | ||
| cd /opt/menagerie && \ | ||
| git sparse-checkout set unitree_g1 && \ | ||
| echo "Menagerie assets installed" | ||
|
|
||
| ENV GENESIS_MENAGERIE_PATH=/opt/menagerie | ||
|
|
||
| # Copy the kinitro environments package (copied by env build command) | ||
| # This is a subset of the main kinitro package - only environments/ is needed | ||
| COPY kinitro /app/kinitro | ||
|
|
||
| # Set PYTHONPATH so kinitro package is importable | ||
| ENV PYTHONPATH="/app:${PYTHONPATH}" | ||
|
|
||
| # Copy the eval environment Actor | ||
| COPY env.py /app/ | ||
|
|
||
| # Affinetes will inject its HTTP server - do not set CMD/ENTRYPOINT |
There was a problem hiding this comment.
Add a non-root USER directive.
Trivy flags DS002: the container runs everything as root. Even though Affinetes manages the container lifecycle, running as a non-root user is a security best practice that limits the blast radius if the container is compromised.
Proposed fix
# Affinetes will inject its HTTP server - do not set CMD/ENTRYPOINT
+
+# Run as non-root for security
+RUN useradd --create-home appuser
+USER appuser📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FROM python:3.11-slim | |
| WORKDIR /app | |
| # Install system dependencies for Genesis rendering (EGL/OpenGL headless) | |
| RUN apt-get update && apt-get install -y --no-install-recommends \ | |
| git \ | |
| # OpenGL/Mesa libraries for headless rendering | |
| libgl1 \ | |
| libgl1-mesa-dri \ | |
| libglib2.0-0 \ | |
| libsm6 \ | |
| libxext6 \ | |
| libxrender1 \ | |
| libgomp1 \ | |
| libegl1 \ | |
| libegl-mesa0 \ | |
| libglvnd0 \ | |
| libglx0 \ | |
| libglx-mesa0 \ | |
| mesa-utils \ | |
| && rm -rf /var/lib/apt/lists/* | |
| # EGL for headless rendering | |
| ENV PYOPENGL_PLATFORM=egl | |
| ENV NVIDIA_DRIVER_CAPABILITIES=all | |
| # Force software rendering when no GPU available | |
| ENV LIBGL_ALWAYS_SOFTWARE=1 | |
| ENV MESA_GL_VERSION_OVERRIDE=3.3 | |
| # Copy requirements and install dependencies | |
| COPY requirements.txt /app/ | |
| RUN pip install --no-cache-dir -r /app/requirements.txt | |
| # Clone MuJoCo Menagerie (sparse checkout -- only robot dirs needed) | |
| # This provides MJCF assets for supported robots (G1, Go2, Franka, etc.) | |
| RUN git clone --depth 1 --filter=blob:none --sparse \ | |
| https://github.com/google-deepmind/mujoco_menagerie.git /opt/menagerie && \ | |
| cd /opt/menagerie && \ | |
| git sparse-checkout set unitree_g1 && \ | |
| echo "Menagerie assets installed" | |
| ENV GENESIS_MENAGERIE_PATH=/opt/menagerie | |
| # Copy the kinitro environments package (copied by env build command) | |
| # This is a subset of the main kinitro package - only environments/ is needed | |
| COPY kinitro /app/kinitro | |
| # Set PYTHONPATH so kinitro package is importable | |
| ENV PYTHONPATH="/app:${PYTHONPATH}" | |
| # Copy the eval environment Actor | |
| COPY env.py /app/ | |
| # Affinetes will inject its HTTP server - do not set CMD/ENTRYPOINT | |
| FROM python:3.11-slim | |
| WORKDIR /app | |
| # Install system dependencies for Genesis rendering (EGL/OpenGL headless) | |
| RUN apt-get update && apt-get install -y --no-install-recommends \ | |
| git \ | |
| # OpenGL/Mesa libraries for headless rendering | |
| libgl1 \ | |
| libgl1-mesa-dri \ | |
| libglib2.0-0 \ | |
| libsm6 \ | |
| libxext6 \ | |
| libxrender1 \ | |
| libgomp1 \ | |
| libegl1 \ | |
| libegl-mesa0 \ | |
| libglvnd0 \ | |
| libglx0 \ | |
| libglx-mesa0 \ | |
| mesa-utils \ | |
| && rm -rf /var/lib/apt/lists/* | |
| # EGL for headless rendering | |
| ENV PYOPENGL_PLATFORM=egl | |
| ENV NVIDIA_DRIVER_CAPABILITIES=all | |
| # Force software rendering when no GPU available | |
| ENV LIBGL_ALWAYS_SOFTWARE=1 | |
| ENV MESA_GL_VERSION_OVERRIDE=3.3 | |
| # Copy requirements and install dependencies | |
| COPY requirements.txt /app/ | |
| RUN pip install --no-cache-dir -r /app/requirements.txt | |
| # Clone MuJoCo Menagerie (sparse checkout -- only robot dirs needed) | |
| # This provides MJCF assets for supported robots (G1, Go2, Franka, etc.) | |
| RUN git clone --depth 1 --filter=blob:none --sparse \ | |
| https://github.com/google-deepmind/mujoco_menagerie.git /opt/menagerie && \ | |
| cd /opt/menagerie && \ | |
| git sparse-checkout set unitree_g1 && \ | |
| echo "Menagerie assets installed" | |
| ENV GENESIS_MENAGERIE_PATH=/opt/menagerie | |
| # Copy the kinitro environments package (copied by env build command) | |
| # This is a subset of the main kinitro package - only environments/ is needed | |
| COPY kinitro /app/kinitro | |
| # Set PYTHONPATH so kinitro package is importable | |
| ENV PYTHONPATH="/app:${PYTHONPATH}" | |
| # Copy the eval environment Actor | |
| COPY env.py /app/ | |
| # Run as non-root for security | |
| RUN useradd --create-home appuser && chown -R appuser:appuser /app | |
| USER appuser | |
| # Affinetes will inject its HTTP server - do not set CMD/ENTRYPOINT |
🤖 Prompt for AI Agents
In `@environments/genesis/Dockerfile` around lines 21 - 76, Add a non-root user in
the Dockerfile, create a dedicated user/group (e.g., "appuser"), chown the
runtime directories (/app and /opt/menagerie) and any files created at build
time, and then switch to that user with a USER directive before the final image
is emitted; specifically: after installing system deps and copying files (after
pip install and the git clone), add commands to create the user/group (useradd
or addgroup/adduser), run chown -R appuser:appuser /app /opt/menagerie (and
adjust ownership of any other runtime paths), and add USER appuser so the
container runs non-root while keeping existing ENV/PYTHONPATH settings intact.
| # Validate env_id is a genesis environment | ||
| if not env_id.startswith("genesis/"): | ||
| return self._build_error_result( | ||
| env_id=env_id, | ||
| task_id=task_id, | ||
| seed=seed or task_id, | ||
| start_time=time.time(), | ||
| error=f"Invalid env_id for Genesis container: {env_id}. Must start with 'genesis/'", | ||
| ) | ||
|
|
||
| if base_url is None: | ||
| raise ValueError("base_url (miner endpoint) is required") | ||
|
|
||
| if seed is None: | ||
| seed = task_id |
There was a problem hiding this comment.
Missing base_url validation raises ValueError outside the try/except block.
Line 145 raises ValueError("base_url (miner endpoint) is required") before the try/except on line 152. This means a None base_url causes a raw exception to propagate to the caller, while all other errors return a structured error dict. This inconsistency could crash the calling framework.
Move the check inside the try/except, or return a structured error result like the env_id check above.
Proposed fix
if base_url is None:
- raise ValueError("base_url (miner endpoint) is required")
+ return self._build_error_result(
+ env_id=env_id,
+ task_id=task_id,
+ seed=seed or task_id,
+ start_time=time.time(),
+ error="base_url (miner endpoint) is required",
+ )🤖 Prompt for AI Agents
In `@environments/genesis/env.py` around lines 134 - 148, The base_url None check
currently raises a raw ValueError before the surrounding try/except and should
instead return a structured error like the env_id check; modify the genesis
environment handler so that when base_url is None it calls
self._build_error_result(...) with the same keys (env_id, task_id, seed or
task_id, start_time=time.time(), and an explanatory error message) or move the
base_url validation inside the existing try block so that any missing-base_url
condition is caught and converted into the structured error result; update the
check near env_id/seed handling that references env_id, base_url, seed, task_id
and uses _build_error_result to ensure consistent error behavior.
| return { | ||
| "task_name": f"robotics:{env_id}", | ||
| "score": score, | ||
| "success": success, | ||
| "time_taken": time.time() - start_time, | ||
| "extra": { | ||
| "task_id": task_id, | ||
| "seed": seed, | ||
| "env_id": env_id, | ||
| "timesteps": timesteps, | ||
| "total_reward": float(total_reward), | ||
| "mean_action_time": float(np.mean(action_times)) if action_times else 0.0, | ||
| "max_action_time": float(np.max(action_times)) if action_times else 0.0, | ||
| "model": model, | ||
| "base_url": base_url, | ||
| }, | ||
| } |
There was a problem hiding this comment.
base_url is included in the result metadata.
Line 308 includes the miner's base_url in the returned extra dict. If miner URLs contain tokens or sensitive deployment details, this could leak information into evaluation logs. Consider whether this is appropriate for your logging/storage pipeline, or omit it.
🤖 Prompt for AI Agents
In `@environments/genesis/env.py` around lines 294 - 310, The returned metadata
currently includes the miner's base_url inside the "extra" dict (key
"base_url"), which may leak sensitive deployment tokens; remove the "base_url"
entry (or replace it with a redacted/boolean flag) from the dict returned by the
function that constructs this result (the block building the "extra" dict
including task_id, seed, env_id, timesteps, total_reward, mean_action_time,
max_action_time, model, base_url) so logs no longer contain the raw URL, and
update any callers/tests that expect base_url accordingly.
|
|
||
| # Available environment families for build command | ||
| AVAILABLE_ENV_FAMILIES = ["metaworld", "procthor"] | ||
| AVAILABLE_ENV_FAMILIES = ["metaworld", "procthor", "genesis"] |
There was a problem hiding this comment.
Update the build_env docstring to include Genesis.
AVAILABLE_ENV_FAMILIES now includes "genesis", but the docstring at lines 57–60 only lists metaworld and procthor under "Environment families". Add a line for Genesis (e.g., description and approximate image size) to keep the help text consistent.
🤖 Prompt for AI Agents
In `@kinitro/cli/env/commands.py` at line 24, The build_env docstring for the
function build_env should be updated to mention the new "genesis" family under
the "Environment families" section; add a short description for Genesis (e.g.,
"Genesis — custom benchmark environments") and an approximate image size similar
to the other entries (e.g., "~1.2GB") so the help text matches
AVAILABLE_ENV_FAMILIES which now includes "genesis".
| try: | ||
| # Update camera pose to follow attached link | ||
| if self._camera._attached_link is not None: | ||
| self._camera.move_to_attach() | ||
|
|
||
| # camera.render() returns (rgb, depth, segmentation, normal) | ||
| rgb, depth, _seg, _normal = self._camera.render(rgb=True, depth=True) | ||
|
|
||
| # Ensure uint8 for RGB | ||
| if rgb is not None and rgb.dtype != np.uint8: | ||
| rgb = (np.clip(rgb, 0, 1) * 255).astype(np.uint8) | ||
|
|
||
| return rgb, depth |
There was a problem hiding this comment.
Two concerns: private attribute access and depth image encoding.
-
_attached_linkis a private attribute (line 477). This couples to Genesis internals and may break on library updates. Consider storing the attachment state locally (e.g., aself._camera_attached: boolflag set in_attach_ego_camera). -
Depth array may not be suitable for
encode_image(line 487). RGB is normalized touint8, but depth is returned raw (likelyfloat32). Ifencode_imageperforms PNG/JPEG encoding expectinguint8, the depth data will be corrupted or cause an error. Verify thatencode_imagehandles float depth arrays correctly, or apply normalization here.
#!/bin/bash
# Check encode_image implementation to see if it handles float32 depth arrays
ast-grep --pattern $'def encode_image($$$) {
$$$
}'
rg -n -A 15 'def encode_image' --type=py| def _generate_object_position( | ||
| self, | ||
| rng: np.random.Generator, | ||
| terrain_type: str, | ||
| terrain_params: dict[str, Any], | ||
| pickupable: bool, | ||
| ) -> list[float]: | ||
| """Generate a valid position for an object, avoiding robot spawn area.""" | ||
| half_arena = self._arena_size / 2.0 | ||
| min_dist_from_center = 0.8 # Avoid robot spawn area | ||
|
|
||
| for _ in range(50): | ||
| x = float(rng.uniform(-half_arena, half_arena)) | ||
| y = float(rng.uniform(-half_arena, half_arena)) | ||
|
|
||
| dist = np.sqrt(x**2 + y**2) | ||
| if dist < min_dist_from_center: | ||
| continue | ||
|
|
||
| # Pickupable objects should be closer (reachable) | ||
| if pickupable and dist > half_arena * 0.7: | ||
| continue | ||
|
|
||
| # Compute z (height) based on terrain | ||
| z = self._get_terrain_height(x, y, terrain_type, terrain_params) | ||
| # Place object on top of terrain | ||
| z += 0.05 # Small offset above ground | ||
|
|
||
| return [x, y, z] | ||
|
|
||
| # Fallback position | ||
| return [1.5, 0.0, 0.05] |
There was a problem hiding this comment.
Fallback position ignores terrain height.
Line 217 returns [1.5, 0.0, 0.05] as a hardcoded fallback, which doesn't call _get_terrain_height. On hilly or stepped terrain, this could place the object below the surface. Given that 50 attempts almost certainly succeed for the configured arena, this is low-risk but easy to fix.
Proposed fix
# Fallback position
- return [1.5, 0.0, 0.05]
+ z = self._get_terrain_height(1.5, 0.0, terrain_type, terrain_params) + 0.05
+ return [1.5, 0.0, z]🤖 Prompt for AI Agents
In `@kinitro/environments/genesis/scene_generator.py` around lines 186 - 217, The
fallback in _generate_object_position currently returns a hardcoded [1.5, 0.0,
0.05] which can place objects below the surface on non-flat terrain; change the
fallback to compute terrain height using _get_terrain_height(1.5, 0.0,
terrain_type, terrain_params) and add the same small offset (0.05) before
returning so the final fallback is [1.5, 0.0, terrain_height + 0.05]; keep the
existing signature and semantics of _generate_object_position unchanged.
| def _get_terrain_height( | ||
| self, | ||
| x: float, | ||
| y: float, | ||
| terrain_type: str, | ||
| terrain_params: dict[str, Any], | ||
| ) -> float: | ||
| """Get terrain height at (x, y) for object placement.""" | ||
| if terrain_type == "flat": | ||
| return 0.0 | ||
| elif terrain_type == "hilly": | ||
| freq = terrain_params.get("frequency", 0.5) | ||
| amp = terrain_params.get("amplitude", 0.2) | ||
| return float(amp * (np.sin(freq * x) + np.cos(freq * y)) * 0.5) | ||
| elif terrain_type == "stepped": | ||
| step_width = terrain_params.get("step_width", 1.0) | ||
| step_height = terrain_params.get("step_height", 0.1) | ||
| dist = np.sqrt(x**2 + y**2) | ||
| step_idx = int(dist / step_width) | ||
| return float(step_idx * step_height) | ||
| return 0.0 |
There was a problem hiding this comment.
Stepped terrain: height model in _get_terrain_height contradicts the geometry built by build_scene.
_get_terrain_height computes height as increasing with distance from center (step_idx = int(dist / step_width) → height grows outward). However, build_scene creates nested boxes that all share origin (0,0) with increasing size and height — so every inner box is overlapped by all outer (taller) boxes. The actual surface height at the center equals step_height * num_steps (the tallest box), while _get_terrain_height returns 0.0 there.
This means objects placed near the center will clip through the stepped geometry (placed at z ≈ 0.05 when the actual surface is at step_height * num_steps), and objects placed further out may also be at incorrect heights.
The fix should either:
- Invert the step geometry in
build_sceneso that inner steps are taller (matching the height model), or - Fix
_get_terrain_heightto match the actual nested-box geometry.
Also applies to: 268-283
🤖 Prompt for AI Agents
In `@kinitro/environments/genesis/scene_generator.py` around lines 219 - 239, The
stepped-terrain height math in _get_terrain_height contradicts the nested-box
geometry created in build_scene: _get_terrain_height currently increases height
with radial distance while build_scene stacks boxes of increasing size (and
height) from center outward, causing center points to be under the tallest outer
box. Fix by making the two match: either modify build_scene to invert the step
stacking so inner boxes are taller (so height decreases with radius), or update
_get_terrain_height to compute heights that match the nested-box geometry (e.g.,
derive step index from box size ordering used in build_scene or compute height
as total_steps - step_idx times step_height). Update the logic in the functions
_get_terrain_height and/or build_scene accordingly and ensure
step_width/step_height params remain consistent between them.
| Returns: | ||
| List of Genesis entity references for object tracking | ||
| """ | ||
| import genesis as gs # noqa: PLC0415 |
There was a problem hiding this comment.
Pipeline failure: genesis import is unresolved.
The CI linter flags import genesis as gs as unresolved. Since Genesis is only available at runtime in the Docker container, consider suppressing the lint with # type: ignore[import-unresolved] in addition to the existing # noqa: PLC0415, or adding a genesis stub/conditional import guard.
Proposed fix
- import genesis as gs # noqa: PLC0415
+ import genesis as gs # noqa: PLC0415 # type: ignore[import-unresolved]🧰 Tools
🪛 GitHub Actions: ci
[error] 251-251: Cannot resolve imported module genesis (unresolved-import) in scene generation. Ensure the Genesis module is available in the Python environment or add it to dependencies.
🤖 Prompt for AI Agents
In `@kinitro/environments/genesis/scene_generator.py` around lines 248 - 251, The
CI linter flags the runtime-only import "import genesis as gs" as unresolved;
fix it by updating that import to suppress the type checker—change the line
"import genesis as gs # noqa: PLC0415" to include a mypy ignore like "import
genesis as gs # noqa: PLC0415 # type: ignore[import-unresolved]" (or
alternatively implement a conditional import guard or small local "genesis"
stub) so the linter stops reporting the unresolved import while keeping the
runtime behavior intact.
| if task_type is None: | ||
| task_type = _random_choice(available_types, rng) | ||
|
|
||
| for _ in range(self._max_attempts): | ||
| if task_type is None: | ||
| return None | ||
| task = self._try_generate_task(objects, rng, task_type, robot_config) | ||
| if task is not None: | ||
| return task | ||
|
|
||
| # Try a different task type | ||
| task_type = _random_choice(available_types, rng) |
There was a problem hiding this comment.
Explicit task_type is silently overridden on retry.
When the caller passes an explicit task_type (e.g., TaskType.PICKUP), the first attempt uses it, but if it fails, line 131 replaces it with a random type from available_types. This means the caller's explicit request is silently ignored after a single failed attempt, potentially returning a task of a completely different type.
If the caller explicitly specified a type, retries should probably stay on that type (varying object selection), or the method should return None if the specified type is infeasible.
Proposed fix
+ # If caller explicitly requested a type, only retry that type
+ explicit_type = task_type is not None
+
if task_type is None:
task_type = _random_choice(available_types, rng)
for _ in range(self._max_attempts):
if task_type is None:
return None
task = self._try_generate_task(objects, rng, task_type, robot_config)
if task is not None:
return task
- # Try a different task type
- task_type = _random_choice(available_types, rng)
+ if not explicit_type:
+ # Try a different task type
+ task_type = _random_choice(available_types, rng)🤖 Prompt for AI Agents
In `@kinitro/environments/genesis/task_generator.py` around lines 120 - 131, The
current loop silently overrides an explicitly passed task_type on retries;
change the logic in the method containing the loop so that if the caller
provided a non-None task_type you do not replace it on subsequent attempts—only
call _random_choice(available_types, rng) when the initial task_type was None.
Concretely: capture a flag like "explicit_type_provided = (task_type is not
None)" before the loop, call self._try_generate_task(objects, rng, task_type,
robot_config) up to self._max_attempts keeping the same task_type when
explicit_type_provided is True, and only re-select task_type from
available_types via _random_choice when explicit_type_provided is False; ensure
the function returns None after exhausting attempts. Reference symbols:
task_type, available_types, _random_choice, _try_generate_task,
self._max_attempts.
| def check_task_feasibility( | ||
| task_type: TaskType, | ||
| target: SceneObject, | ||
| destination: SceneObject | None = None, | ||
| robot_supported_tasks: list[str] | None = None, | ||
| ) -> tuple[bool, str]: | ||
| """ | ||
| Check if a task is feasible given the target object and robot capabilities. | ||
|
|
||
| Returns: | ||
| Tuple of (is_feasible, reason) | ||
| """ | ||
| # Check robot supports this task type | ||
| if robot_supported_tasks is not None: | ||
| if task_type.value not in robot_supported_tasks: | ||
| return False, f"Robot does not support task type {task_type.value}" | ||
|
|
||
| # Check required properties | ||
| required_props = TASK_REQUIRED_PROPERTIES[task_type] | ||
| for prop in required_props: | ||
| if not getattr(target, prop, False): | ||
| return False, f"Object {target.object_type} is not {prop}" | ||
|
|
||
| # Task-specific checks | ||
| if task_type == TaskType.PICKUP: | ||
| if target.is_picked_up: | ||
| return False, f"Object {target.object_type} is already picked up" | ||
|
|
||
| if task_type == TaskType.PLACE: | ||
| if destination is None: | ||
| return False, "Place task requires a destination" | ||
|
|
||
| if task_type == TaskType.PUSH: | ||
| if destination is None: | ||
| return False, "Push task requires a destination" | ||
| if target.object_id == destination.object_id: | ||
| return False, "Cannot push object to itself" | ||
|
|
||
| if task_type == TaskType.NAVIGATE: | ||
| # Navigate is always feasible if target exists | ||
| pass | ||
|
|
||
| return True, "Task is feasible" |
There was a problem hiding this comment.
PLACE feasibility doesn't guard against target == destination (unlike PUSH).
Lines 136–140 validate that PUSH requires destination is not None and target.object_id != destination.object_id. The PLACE check (lines 132–134) only validates the destination exists but doesn't prevent placing an object at its own location. While the current TaskGenerator naturally prevents this (it picks from disjoint pickupable/non-pickupable sets), the feasibility function should be self-consistent.
Proposed fix
if task_type == TaskType.PLACE:
if destination is None:
return False, "Place task requires a destination"
+ if target.object_id == destination.object_id:
+ return False, "Cannot place object at itself"🤖 Prompt for AI Agents
In `@kinitro/environments/genesis/task_types.py` around lines 104 - 146,
check_task_feasibility currently verifies destination for TaskType.PLACE but
doesn't guard against target == destination; update the TaskType.PLACE branch in
check_task_feasibility to mirror the PUSH checks by returning infeasible when
destination is None or when target.object_id == destination.object_id (include a
clear reason string like "Cannot place object at its own location"), referencing
the TaskType.PLACE branch and the target/destination object_id comparisons to
locate the change.
Shr1ftyy
force-pushed
the
feat/genesis
branch
3 times, most recently
from
af55012 to
846280e
Compare
Adds a new environment family powered by the Genesis simulation engine, with a framework-based architecture where GenesisBaseEnvironment handles all boilerplate (engine init, scene building, first-person ego camera, PD control pipeline) and thin subclasses define per-robot behavior. First concrete environment: Unitree G1 humanoid (43 actuated DOFs) with procedural scene generation (flat ground, 3-6 primitive objects) and four task types (navigate, pickup, place, push). Includes CLI --viewer and --seed flags for `kinitro env test`, registry **kwargs passthrough for env factory options, and 72 unit tests covering all pure-logic modules.
Shr1ftyy
force-pushed
the
feat/genesis
branch
from
846280e to
25a6c8e
Compare
1 participant
Summary
genesisenvironment family powered by the Genesis physics simulation engineGenesisBaseEnvironmenthandles all boilerplate (engine init, scene building, first-person ego camera attached to robot torso, PD control pipeline), thin subclasses define per-robot behaviorkinitro/genesis:v1) serves allgenesis/*env_idsgenesis/g1-v0in the environment registry, CLI, and executor configNew files
kinitro/environments/genesis/— task types, robot config, scene generator, task generator, base environment, G1 subclassenvironments/genesis/— Dockerfile, Actor (env.py), metadata, requirementsTest plan
uv run python -m ruff checkpassesuv run python -m pytest tests/ -x -qpasses (116 tests)kinitro env build genesisbuilds Docker image successfullySummary by CodeRabbit
Release Notes
New Features
Infrastructure