Document Kamino solver presets#5457
Conversation
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR adds documentation for the Kamino solver preset system, explaining how users can select between MuJoCo-Warp and Kamino solvers through the existing Hydra preset mechanism. The changes also update outdated API references in the solver-transitioning documentation to reflect current public APIs. This is a documentation-only change with no code modifications.
Architecture Impact
Self-contained. This PR only modifies .rst documentation files and does not affect any runtime code paths.
Implementation Verdict
Ship it — with one minor documentation accuracy concern to verify.
Test Coverage
N/A — Documentation-only change. No tests required or expected.
CI Status
No CI checks available yet. Documentation builds should be verified.
Findings
🔵 Improvement: docs/source/features/hydra.rst:304-306 — Verify command line path accuracy
The example shows env.sim.physics=kamino but earlier in the same file (line 263), the CartpolePhysicsCfg is shown as a field directly under the environment config, not under env.sim. The correct path depends on how the actual task configs are structured. If physics is defined as env.backend or env.physics rather than env.sim.physics, this example could confuse users.
# Current
python train.py --task=Isaac-Cartpole-v0 env.sim.physics=kamino
# Verify this matches actual task config structure🔵 Improvement: docs/source/features/hydra.rst:280-282 — Clarify "currently beta" vs "experimental"
The documentation uses both "currently beta" (line 281) and "experimental" (line 288 note). Consider using consistent terminology throughout both files. The solver-transitioning.rst uses "experimental" and "beta support" in close proximity as well (lines 4, 8). Pick one term for consistency.
🔵 Improvement: docs/source/experimental-features/newton-physics-integration/solver-transitioning.rst:5-6 — Cross-reference could be more discoverable
The cross-reference to :ref:hydra-backend-solver-presets`` is good, but users navigating from the experimental-features section might not realize the main Kamino documentation lives in the Hydra features page. Consider adding a brief sentence like "For full configuration examples and usage, see..." to make the primary documentation location clearer.
🔵 Improvement: docs/source/features/hydra.rst:268-272 — KaminoSolverCfg parameters not explained
The KaminoSolverCfg parameters (integrator, use_collision_detector, sparse_jacobian, padmm_max_iterations) are shown in the example but not explained anywhere. Unlike the MuJoCo-Warp parameters documented in solver-transitioning.rst, Kamino's parameters have no accompanying explanations. Users won't know what "moreau" integrator means or when to tune padmm_max_iterations.
Consider adding brief inline comments or a note pointing to where users can learn about these parameters:
solver_cfg=KaminoSolverCfg(
integrator="moreau", # Integration method
use_collision_detector=True,
sparse_jacobian=True, # Performance optimization
padmm_max_iterations=100, # Solver convergence iterations
),🔵 Improvement: docs/source/experimental-features/newton-physics-integration/solver-transitioning.rst:26-27 — Import statement includes unused SimulationCfg
The example imports SimulationCfg from isaaclab.sim, which is correct, but the full file context shows line 46 uses it. This is fine, but verify the import path isaaclab.sim.SimulationCfg is the canonical public API (vs. the old private _impl paths that were removed).
Overall, this is clean documentation work that correctly updates deprecated API paths and provides clear explanations of the preset system for solver selection. The changes are accurate and well-structured.
Greptile SummaryThis documentation-only PR adds a "Backend and Solver Presets" section to Confidence Score: 4/5Safe to merge; documentation-only change with no runtime impact All findings are P2 style suggestions. The code snippets reference correct public APIs, cross-references are valid, and the new Kamino section is accurate. One minor duplication of the experimental note across two files, but it does not cause any functional concern. No files require special attention Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
CLI[CLI argument\ne.g. presets=kamino] --> PR[Preset resolver]
PR -->|preset name = physx| PhysxCfg[PhysxCfg\nPhysX backend]
PR -->|preset name = newton| NewtonMJWarp[NewtonCfg\nsolver=MJWarpSolverCfg]
PR -->|preset name = kamino| NewtonKamino[NewtonCfg\nsolver=KaminoSolverCfg]
PR -->|no match / default| Default[default field\nPhysxCfg]
NewtonMJWarp --> Newton[Newton backend]
NewtonKamino --> Newton
PhysxCfg --> PhysX[PhysX backend]
Default --> PhysX
Reviews (1): Last reviewed commit: "Document Kamino solver presets" | Re-trigger Greptile |
| .. note:: | ||
|
|
||
| Kamino support is experimental and currently depends on assets being structured | ||
| in a way that Kamino can consume. Assets that work with MuJoCo-Warp or PhysX | ||
| may still require model-structure updates before they work with Kamino. |
There was a problem hiding this comment.
The Kamino experimental note on lines 10–14 is nearly identical to the one added in hydra.rst (lines 292–297). Since solver-transitioning.rst already cross-references the Hydra presets page via :ref:\hydra-backend-solver-presets`, readers will encounter the same caveat twice in close succession. Consider removing the inline note here and relying solely on the one in hydra.rst`, or replacing it with a forward-reference phrase (e.g. "see the note in the linked section") to avoid the duplication.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| Kamino is therefore a solver preset, not a separate Isaac Lab backend. The same | ||
| Newton assets, sensors, renderers, and visualizers are used after the preset is | ||
| resolved. It is a Proximal Alternating Direction Method of Multipliers (P-ADMM) | ||
| based solver for constrained rigid multi-body dynamics, and its Isaac Lab support | ||
| is currently beta. |
There was a problem hiding this comment.
"Beta" vs "experimental" terminology inconsistency
The prose on line 290 describes Kamino support as "currently beta", while both the .. note:: block immediately below and the corresponding note in solver-transitioning.rst use the term "experimental". These two terms carry different connotations — "beta" implies a more mature, near-release state; "experimental" implies early/unstable. Aligning them to a single term avoids reader confusion about the actual stability level.
| The ``kamino`` preset is currently defined for ``Isaac-Cartpole-Direct-v0``, | ||
| ``Isaac-Ant-Direct-v0``, ``Isaac-Cartpole-v0``, and ``Isaac-Ant-v0``. Passing | ||
| ``presets=kamino`` to a task without a ``kamino`` preset does not enable Kamino; | ||
| add and validate a task-specific preset first. |
There was a problem hiding this comment.
Hardcoded task list will go stale
The enumerated set of tasks that support the kamino preset (Isaac-Cartpole-Direct-v0, Isaac-Ant-Direct-v0, Isaac-Cartpole-v0, Isaac-Ant-v0) is embedded directly in the docs. As tasks are added or removed, this list will silently diverge from reality. Consider pointing to a discoverable location (a registry call, a grep pattern, or a separate changelog) or adding a comment reminding maintainers to keep this list in sync.
| kamino: NewtonCfg = NewtonCfg( | ||
| solver_cfg=KaminoSolverCfg( | ||
| integrator="moreau", | ||
| use_collision_detector=True, | ||
| sparse_jacobian=True, | ||
| padmm_max_iterations=100, | ||
| ), | ||
| num_substeps=1, | ||
| debug_mode=False, | ||
| use_cuda_graph=True, | ||
| ) |
There was a problem hiding this comment.
Kamino-specific parameters have no inline explanation
The KaminoSolverCfg snippet introduces four parameters (integrator, use_collision_detector, sparse_jacobian, padmm_max_iterations) without any descriptive bullet list, while the MJWarp parameters (njmax, nconmax, ls_iterations, etc.) are all explained in detail nearby in solver-transitioning.rst. A short explanation or cross-reference to API docs for these Kamino parameters would help users tune the solver without hunting through separate references.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
| Kamino support is experimental and currently depends on assets being structured | ||
| in a way that Kamino can consume. Assets that work with MuJoCo-Warp or PhysX | ||
| may still require model-structure updates before they work with Kamino. |
There was a problem hiding this comment.
could we have another tutorial to describe what changes are needed to work with kamino? and maybe some descriptions of the kamino-specific solver parameters
# Description Adds a dedicated Newton experimental tutorial for using the Kamino solver. The page explains that Kamino is selected through a Newton physics solver preset, shows the task changes needed to add a `kamino` preset, lists compatibility checks for assets, resets, sensors, and renderers, and documents the Kamino-specific solver parameters by category. This addresses Kellys follow-up request on #5457 for a tutorial describing what needs to change to work with Kamino and for descriptions of Kamino-specific solver parameters. Fixes # (issue) ## Type of change - Documentation update ## Screenshots Not applicable. Documentation-only RST change. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have updated the changelog and the corresponding version in the extensions `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there Notes: - Ran `./isaaclab.sh -f` successfully. - Verified the `literalinclude` target and referenced labels locally. - `./isaaclab.sh -d` could not start in this checkout because there is no virtual environment and system Python is PEP 668 protected, so pip refused to install docs requirements. Because Sphinx did not run, the warning checklist item is intentionally left unchecked. - No tests or changelog fragment were added because this is a documentation-only follow-up under `docs/source`; the current repository guidance uses `source/<pkg>/changelog.d` fragments only for touched source packages.
Description
Document Kamino solver selection through the existing Hydra preset system.
newtonandkaminopresets both useNewtonCfgbut choose different solver configs.Fixes: N/A
Type of change
Screenshots
N/A. Documentation-only change.
Checklist
pre-commitchecks with./isaaclab.sh --formatCONTRIBUTORS.mdor my name already exists there