Add solver documentation guide - #3325
Conversation
Move solver selection and support guidance out of the generated API page into a dedicated Solvers section. Rehome the MuJoCo guide, add a concise Kamino page, preserve legacy links, and expose Isaac Lab directly in navigation.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds a Solvers guide, adds a new Isaac Lab page under Lab, redirects old Integrations pages to the new locations, and updates documentation links plus inline MuJoCo references to the reorganized doc paths. ChangesDocumentation reorganization
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/solvers/index.rst`:
- Around line 145-146: The MuJoCo footnote wording is reversed and should match
the detailed docs: update the note in the solver overview so it states that
MuJoCo uses its own collision pipeline by default, and that setting
use_mujoco_contacts to False switches to Newton contacts. Keep the change in the
same note near the MuJoCo solver entry so the overview aligns with the dedicated
MuJoCo guide.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 739e56d4-2ca3-4d8b-8eda-296ede187aee
📒 Files selected for processing (18)
CHANGELOG.mddocs/api/newton_solvers.rstdocs/concepts/collisions.rstdocs/guide/installation.rstdocs/guide/overview.rstdocs/index.rstdocs/integrations/index.rstdocs/integrations/isaac-lab.rstdocs/integrations/mujoco.rstdocs/lab/isaac-lab.rstdocs/solvers/index.rstdocs/solvers/kamino.rstdocs/solvers/mujoco.rstnewton/_src/solvers/mujoco/kernels.pynewton/_src/solvers/mujoco/solver_mujoco.pynewton/_src/usd/schemas.pynewton/_src/utils/import_mjcf.pynewton/solvers.py
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Keep the legacy MuJoCo redirect. Carry upstream equality guidance into the canonical solver page.
adenzler-nvidia
left a comment
There was a problem hiding this comment.
The migration itself checks out: the moves are verbatim, the use_mujoco_contacts correction matches the code default, and the hand-edited docs/api/newton_solvers.rst matches what docs/generate_api.py regenerates. Two referrers outside this diff still point at the old location, though:
-
docs/tutorials/00_introduction.ipynblinks tohttps://newton-physics.github.io/newton/stable/api/newton_solvers.html#supported-features, but this PR removes the Supported Features section from that page. The redirect stubs can't cover this case since the page itself still exists, so tutorial readers will land on the slimmed-down API page with no feature table. The link should target the new anchor on/solvers/index.html(the PR already adds an explicit.. _Supported Features:label there). -
docs/conf.py'ssecondary_sidebar_itemsoverride still disables the right-hand page TOC only forapi/newton_solvers, with the comment citing "several wide comparison tables" — but those tables moved tosolvers/index. As is, the new overview renders the wide feature/joint-support tables with the secondary sidebar eating content width (the problem the override was added to fix), while the now-slim API page needlessly loses its page TOC.
Clarify solver guidance and align moved pages with their new locations. Update stale links and preserve source and wheel discoverability.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
Addressed the remaining review-summary findings in a815f00:
The four inline threads addressed by the same commit have also been replied to and resolved. |
Description
Add a top-level Solvers documentation section so solver selection, feature comparisons, and backend-specific guidance are easier to discover.
newton.solversmodule docstring.This is a documentation-only change; it does not change solver behavior or public APIs.
Closes #2257
Content migration and discoverability
This PR reorganizes existing documentation rather than removing it:
newton/solvers.py—including the solver workflow, feature matrix, contact-material support, joint-feature support, and differentiability guidance—moves todocs/solvers/index.rst.docs/integrations/mujoco.rstmoves todocs/solvers/mujoco.rst. Its content is unchanged except for correcting the inaccurate statement thatuse_mujoco_contacts=Falseis the default.docs/integrations/isaac-lab.rstmoves todocs/lab/isaac-lab.rst.docs/api/newton_solvers.rstbecomes API-focused; its module declaration, submodule navigation, and solver autosummary remain unchanged.A key design consideration was whether moving the large
newton.solversmodule docstring would make the information harder to discover for source-first users and coding agents. To preserve that discoverability without maintaining duplicate feature matrices, the shortened module docstring now:docs/solvers/index.rst;The detailed information therefore has one authoritative home, while remaining reachable from the source module, generated API page, top-level documentation sidebar, overview page, and hosted documentation.
Checklist
CHANGELOG.mdhas been updated (if user-facing change)Test plan
The strict HTML build passed, all 85 doctests passed, and the rendered sidebar and legacy URL outputs were inspected directly.
Summary by CodeRabbit