Add Kamino solver tutorial#5483
Conversation
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR adds documentation for using the Kamino solver within the Newton physics integration. It introduces a new tutorial page (using-kamino.rst) explaining how to enable Kamino through task physics presets, documents solver parameters, and provides a tuning workflow. The changes are purely documentation additions with no code modifications.
Architecture Impact
Self-contained. This is a documentation-only PR that adds a new RST file and updates the toctree/cross-references in existing documentation. No code paths are affected.
Implementation Verdict
Minor fixes needed
Test Coverage
Not applicable for documentation-only changes. The author notes they verified the literalinclude target and referenced labels locally, which is the appropriate validation for RST files.
CI Status
No CI checks available yet. For documentation PRs, the key validation would be that Sphinx builds successfully without warnings, which the author could not verify due to PEP 668 environment restrictions (noted in PR description).
Findings
🟡 Warning: using-kamino.rst:45-48 — Potential broken literalinclude reference
The literalinclude directive references:
.. literalinclude:: ../../../../source/isaaclab_tasks/isaaclab_tasks/direct/cartpole/cartpole_env_cfg.py
:start-at: class CartpolePhysicsCfg
:end-before: class CartpoleEnvCfgWhile the author claims to have verified this locally, I cannot validate that class CartpolePhysicsCfg actually exists in the target file with a kamino preset at lines 18-37 as the emphasize-lines suggests. If this class or the kamino preset doesn't exist yet, the docs build will fail or show incorrect content. Consider adding a note in the PR description confirming the target file has been updated with the Kamino preset, or ensure this PR is merged after/with the code PR that adds it.
🔵 Improvement: using-kamino.rst:7-9 — Consider adding version/availability note
The documentation references :class:~isaaclab_newton.physics.NewtonCfg and `:class:`~isaaclab_newton.physics.KaminoSolverCfg without indicating which Isaac Lab version introduced these classes. Since Kamino is described as "beta," adding a .. versionadded:: directive would help users understand feature availability.
🔵 Improvement: using-kamino.rst:31-34 — Hardcoded task list may become stale
The list of tasks with Kamino presets (Isaac-Cartpole-Direct-v0, Isaac-Ant-Direct-v0, etc.) is hardcoded. Consider either:
- Adding a note that this list may not be exhaustive
- Pointing users to a programmatic way to discover which tasks support Kamino (e.g., grep for
kaminoin task configs)
🔵 Improvement: using-kamino.rst:110-118 — Missing default values in parameter tables
The parameter tables document descriptions but not default values. For tuning purposes, knowing defaults (e.g., what "euler" vs "moreau" means for integrator, or the default padmm_max_iterations) would significantly help users understand what the validated presets actually set. Consider adding a "Default" column or mentioning defaults inline.
🔵 Improvement: using-kamino.rst:22-28 — Command syntax may be confusing
The commands show --viz newton presets=newton where --viz newton is a flag and presets=newton appears to be a Hydra override. The lack of -- or other separator between argparse flags and Hydra overrides may confuse users. Consider adding a brief note clarifying this syntax, or using the more explicit +presets=newton if that's the correct Hydra syntax.
🔵 Improvement: solver-transitioning.rst:8 — Cross-reference ordering
The updated line reads:
separate backend; see :ref:`hydra-backend-solver-presets` and :ref:`newton-using-kamino`.This is fine, but consider whether the Kamino reference should come first since it's more specific to the sentence's context (about Kamino being a preset rather than a backend).
Greptile SummaryThis PR adds a new Confidence Score: 4/5Safe to merge; only P2 documentation style issues found, no runtime or correctness impact. All findings are P2 style issues around the literalinclude snippet boundaries. Cross-references and file paths resolve correctly. docs/source/experimental-features/newton-physics-integration/using-kamino.rst — review the literalinclude boundaries around the import and trailing decorator. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User: presets=kamino] --> B{Task defines kamino preset?}
B -- No --> C[Kamino not enabled]
B -- Yes --> D[CartpolePhysicsCfg.kamino\nNewtonCfg + KaminoSolverCfg]
D --> E[Newton model built from USD assets]
E --> F{Newton model OK?}
F -- No --> G[Fix asset/task config first\npresets=newton must pass]
F -- Yes --> H[Kamino solver selected\nP-ADMM + FK-reset path]
H --> I[Tune: num_substeps\npadmm_max_iterations\nP-ADMM tolerances]
I --> J[Stable task ready for training]
Reviews (1): Last reviewed commit: "Add Kamino solver tutorial" | Re-trigger Greptile |
| .. literalinclude:: ../../../../source/isaaclab_tasks/isaaclab_tasks/direct/cartpole/cartpole_env_cfg.py | ||
| :language: python | ||
| :start-at: class CartpolePhysicsCfg | ||
| :end-before: class CartpoleEnvCfg | ||
| :emphasize-lines: 18-37 | ||
|
|
||
| The important pieces are: | ||
|
|
||
| * Import :class:`~isaaclab_newton.physics.KaminoSolverCfg`. |
There was a problem hiding this comment.
Literalinclude snippet omits the import statement
The literalinclude directive uses :start-at: class CartpolePhysicsCfg, which begins at line 24 of cartpole_env_cfg.py. This skips the import block at the top of the file, so the rendered snippet will not show:
from isaaclab_newton.physics import KaminoSolverCfg, MJWarpSolverCfg, NewtonCfgHowever, the very next paragraph explicitly lists "Import KaminoSolverCfg" as one of the important pieces. Readers copying from the snippet won't see where to put that import. Consider adding a separate short code block showing the import, or extending the range to include the import lines.
| .. literalinclude:: ../../../../source/isaaclab_tasks/isaaclab_tasks/direct/cartpole/cartpole_env_cfg.py | ||
| :language: python | ||
| :start-at: class CartpolePhysicsCfg | ||
| :end-before: class CartpoleEnvCfg | ||
| :emphasize-lines: 18-37 |
There was a problem hiding this comment.
Snippet ends with a dangling decorator from the next class
:end-before: class CartpoleEnvCfg stops just before class CartpoleEnvCfg(DirectRLEnvCfg): (file line 66), but line 65 is @configclass — the decorator for that next class. The rendered snippet will close with a bare @configclass on its own, which may confuse readers into thinking it belongs to CartpolePhysicsCfg. Consider using :end-before: @configclass instead, or specify an explicit :lines: range.
7c8dac7 to
d85fc21
Compare
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This follow-up addresses several concerns from the previous review. The author has added default values inline in all parameter table descriptions, which resolves the most significant documentation usability issue.
Previous Findings Status
- ✅ Default values in parameter tables — Addressed. All parameters now include inline defaults (e.g.,
Default: ``"euler"```,Default: ``200```, etc.). ⚠️ literalinclude reference — Partially addressed. The:end-at:marker was changed from:end-before: class CartpoleEnvCfgto:end-at: ovphysx: OvPhysxCfg = OvPhysxCfg(), which is more specific but still depends on the target file having theCartpolePhysicsCfgclass with the Kamino preset.- Remaining items (version note, hardcoded task list, command syntax clarification, cross-reference ordering) were minor improvements and not addressed, which is acceptable.
Implementation Verdict
Ship it
New Findings
No new issues introduced. The documentation is comprehensive, the parameter defaults are now well-documented, and the tuning workflow provides clear guidance for users adopting Kamino.
|
@kellyguo11 Tutorial for Kamino. |
|
|
||
| ./isaaclab.sh -p scripts/environments/zero_agent.py --task Isaac-Cartpole-Direct-v0 --num_envs 128 --viz newton presets=kamino | ||
|
|
||
| At the time of writing, the ``kamino`` preset is defined for |
There was a problem hiding this comment.
Question: Could we simplify this, i.e. to automate most of the Kamino-specific adaptions? If yes, the doc could mention this.
| Kamino uses the Newton model built from the task assets. When adding Kamino to a | ||
| new task, validate the following before tuning solver parameters: | ||
|
|
||
| * The task must already be compatible with the Newton backend. If ``presets=newton`` |
There was a problem hiding this comment.
This sounds as if Kamino were different from Newton
| - Default: ``1e-6``. Primal residual convergence tolerance. | ||
| * - ``padmm_dual_tolerance`` | ||
| - Default: ``1e-6``. Dual residual convergence tolerance. | ||
| * - ``padmm_compl_tolerance`` | ||
| - Default: ``1e-6``. Complementarity residual convergence tolerance for contacts and unilateral constraints. | ||
| * - ``padmm_rho_0`` |
There was a problem hiding this comment.
I recommend looser tolerances for these, e.g. closer to 1e-4, unless accuracy is particularly important.
|
@kellyguo11 after chatting with @camevor it's quite we need to rename our current newton preset to mjwarp. |
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This is a documentation-only PR adding a Kamino solver tutorial. The previous review's main concern (missing default values in parameter tables) was addressed in an earlier commit. No new changes since that fix.
Implementation Verdict
Ship it
Findings
No new issues. The documentation is comprehensive, well-structured, and provides clear guidance for users adopting the Kamino solver. The inline defaults in parameter tables, cross-references to related docs, and step-by-step tuning workflow are all solid.
# Description Renames the public Newton MJWarp physics preset from ``newton`` to ``mjwarp`` to distinguish solver selection from the Newton backend, Newton package, and Newton visualizer names. The change keeps legitimate Newton component names intact, including ``isaaclab_newton``, ``newton_renderer``, ``--viz newton``, backend label ``newton``, and solver value ``"newton"``. Legacy ``newton`` preset selections now resolve to ``mjwarp`` with a deprecation warning. Related to #5483. No new dependencies are required. ## Type of change - Bug fix (non-breaking change which fixes an issue) - Documentation update ## Screenshots Not applicable. ## Testing - Docker: ``./isaaclab.sh -p -m pytest source/isaaclab_tasks/test/test_mjwarp_preset_names.py source/isaaclab_tasks/test/test_hydra.py source/isaaclab_tasks/test/test_preset_kit_decision.py -q``: 74 passed - Docker: ``./isaaclab.sh -p -m pytest source/isaaclab_tasks/test/test_shadow_hand_vision_presets.py -q``: 67 passed, 6 skipped, 2 xpassed - Docker: ``./isaaclab.sh -p -m pytest "source/isaaclab_visualizers/test/test_visualizer_cartpole_integration.py::test_cartpole_newton_visualizer_tiled_camera_rgb_non_black[newton]" -q``: 1 passed - Docker: ``./isaaclab.sh -p -m pytest --collect-only -q source/isaaclab_tasks/test/test_environments_newton.py``: 86 tests collected - Docker byte-compile of changed Python files: passed - Docker grep guards for stale public ``newton`` preset references: passed - Host: ``./isaaclab.sh -f``: passed - Host: ``git diff --check``: passed ## 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 - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there Note: this PR adds changelog fragments under ``changelog.d``. The extension version is compiled by the nightly workflow, so ``config/extension.toml`` is intentionally not edited directly.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
Follow-up review on documentation-only PR. No new commits since the previous review — the diff and file contents are identical to the last review cycle.
Implementation Verdict
Ship it
Findings
Previous concern about missing default values was already addressed. No new changes to review. The documentation is comprehensive and ready to merge.
Update preset references in the Kamino tutorial after the rename in isaac-sim#5488 ("Rename Newton preset to MJWarp"): * ``presets=newton`` -> ``presets=newton_mjwarp`` * ``presets=kamino`` -> ``presets=newton_kamino`` * ``env.sim.physics=kamino`` -> ``env.sim.physics=newton_kamino`` * Backticked preset name references in prose updated to match. This brings the tutorial in line with the renamed preset keys and satisfies the regression check in ``test_newton_solver_preset_names.py``.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
No new commits since the previous review — the diff and file contents remain identical. The documentation for the Kamino solver tutorial is comprehensive, well-structured, and ready to merge.
Implementation Verdict
Ship it
Findings
No new issues. Previous review already approved this documentation-only change. The literalinclude path and cross-references have been verified locally by the author.
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
kaminopreset, 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
Screenshots
Not applicable. Documentation-only RST change.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists thereNotes:
./isaaclab.sh -fsuccessfully.literalincludetarget and referenced labels locally../isaaclab.sh -dcould 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.docs/source; the current repository guidance usessource/<pkg>/changelog.dfragments only for touched source packages.