Document viewer debugging workflows - #3718
Conversation
Make current viewer-based debugging easier to discover. Clarify ViewerFile recording contents and playback limitations.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDocumentation updates rename and expand viewer debugging guidance, describe ViewerFile recording and replay behavior, improve navigation and FAQ references, and clarify collision geometry and contact-normal validation. ChangesViewer debugging documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guide/visualization.rst (1)
341-346: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdvance the state in the recording example.
The loop only increments
sim_time; it logs the samestatefive times. Playback therefore contains identical snapshots and does not demonstrate recorded simulation motion. Step a solver/updatestatebetween frames, or explicitly label this as a static snapshot example.🤖 Prompt for 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. In `@docs/guide/visualization.rst` around lines 341 - 346, Update the recording loop in the visualization example to advance or mutate state between calls to viewer.log_state, using the existing solver or state-update mechanism if available, so each frame records a distinct simulation snapshot; only label it static if no state update is intended.
🤖 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/guide/visualization.rst`:
- Around line 382-386: Update the recording/replay workflow in the visualization
guide around the `uv run -m newton.examples recording` and `replay_viewer`
commands so it installs or otherwise includes the required `cbor2` dependency
before creating `humanoid_recording.bin`; keep the binary recording workflow
consistent with the documented prerequisite.
---
Outside diff comments:
In `@docs/guide/visualization.rst`:
- Around line 341-346: Update the recording loop in the visualization example to
advance or mutate state between calls to viewer.log_state, using the existing
solver or state-update mechanism if available, so each frame records a distinct
simulation snapshot; only label it static if no state update is intended.
🪄 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 Plus
Run ID: ccb00aa1-263c-4d74-bc05-b8a6a8421a57
📒 Files selected for processing (4)
docs/concepts/simulation_tuning.rstdocs/faq.rstdocs/guide/overview.rstdocs/guide/visualization.rst
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Run both recording examples with the examples extra so binary recording and ViewerGL dependencies are available.
gyeomannvidia
left a comment
There was a problem hiding this comment.
Looks good! I made a couple of really nit-picky comments. You can decide whether to modify or not.
Address review feedback by distinguishing the static API snippet and describing the multi-world humanoid example precisely.
|
@gyeomannvidia Thanks again for the review. I addressed the review comments in 62dcb9a, replied in each thread, and resolved the remaining threads. The latest push dismissed the earlier approval; could you please re-approve when convenient? |
Description
Rename the visualization guide to Viewers and Debugging and make Newton's
current viewer-based debugging workflows easier to discover.
The updated guide:
state-snapshot recording, and persistent visual or scene export;
log_*()calls;ViewerFilecaptures and what it does not;This gives users an accurate entry point for Newton's current debugging
capabilities without promising future diagnostic functionality.
Closes #3295
Checklist
CHANGELOG.mdhas been updated (not required for this documentation-only change)Test plan
Summary by CodeRabbit