Skip to content

Bugfix: Add default joint friction value in USD parser#1018

Merged
eric-heiden merged 3 commits into
newton-physics:mainfrom
Kenny-Vilella:dev/kvilella/bugfix_friction_default
Oct 31, 2025
Merged

Bugfix: Add default joint friction value in USD parser#1018
eric-heiden merged 3 commits into
newton-physics:mainfrom
Kenny-Vilella:dev/kvilella/bugfix_friction_default

Conversation

@Kenny-Vilella

@Kenny-Vilella Kenny-Vilella commented Oct 31, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Description

  • Added missing default joint friction value in USD parser

Newton Migration Guide

Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.

  • The migration guide in docs/migration.rst is up-to date

Before your PR is "Ready for review"

  • Necessary tests have been added and new examples are tested (see newton/tests/test_examples.py)
  • Documentation is up-to-date
  • Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

  • Bug Fixes
    • Joint friction now uses configurable defaults instead of hard-coded values, enabling more consistent physics simulation behavior across imported assets.

@coderabbitai

coderabbitai Bot commented Oct 31, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Modified the default joint friction behavior in the USD import utility by replacing a hard-coded default value of 0.0 with a configurable default sourced from the builder's default joint configuration, affecting friction assignment for revolute and prismatic joints.

Changes

Cohort / File(s) Change Summary
Joint friction default configuration
newton/_src/utils/import_usd.py		 Introduced local variable default_joint_friction assigned from builder.default_joint_cfg.friction to replace hard-coded default of 0.0 when parsing friction attributes on joints

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify that builder.default_joint_cfg.friction is reliably accessible at the point of assignment
  • Confirm the change applies consistently to all affected joint types (revolute/prismatic)
  • Validate that existing behavior is preserved for joints that explicitly specify friction values

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Bugfix: Add default joint friction value in USD parser" directly and clearly describes the main change in the pull request. The change introduces a new default joint friction value in the USD parser by using builder.default_joint_cfg.friction instead of a hard-coded default of 0.0. The title is concise, specific, and properly categorizes this as a bugfix. A team member scanning the commit history would immediately understand that this PR addresses a missing default friction value in the USD parsing logic.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e556064 and 4952ba6.

📒 Files selected for processing (1)
  • newton/_src/utils/import_usd.py (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
newton/_src/utils/import_usd.py (1)
newton/_src/utils/schema_resolver.py (3)
  • get_value (91-109)
  • get_value (660-708)
  • PrimType (34-42)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Run GPU Benchmarks (Pull Request)
  • GitHub Check: Run GPU Unit Tests on AWS EC2 (Pull Request)
🔇 Additional comments (2)
newton/_src/utils/import_usd.py (2)

157-157: LGTM! Consistent pattern for default joint parameters.

The addition of default_joint_friction follows the same pattern as other joint defaults (lines 158-160), making the code more maintainable and consistent.

602-604: Correct implementation, but verify migration guide update.

The change correctly uses default_joint_friction as the default parameter, which aligns with the schema resolver's precedence logic. However, this modifies the default behavior for joints that don't have friction explicitly authored in the USD.

The PR objectives indicate that the migration guide needs updating (currently unchecked). Please ensure that the migration guide documents this change, especially if builder.default_joint_cfg.friction differs from the previous hard-coded default of 0.0, as this could affect existing USD files that rely on the implicit default friction value.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Oct 31, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@Kenny-Vilella

Kenny-Vilella commented Oct 31, 2025

Copy link
Copy Markdown
Member Author

The failing benchmark is a bit surprising.
I will look on Monday what is happening.

@eric-heiden eric-heiden merged commit 46b8254 into newton-physics:main Oct 31, 2025
15 of 16 checks passed
@Kenny-Vilella Kenny-Vilella deleted the dev/kvilella/bugfix_friction_default branch November 3, 2025 00:39
eric-heiden added a commit to eric-heiden/newton that referenced this pull request Jan 28, 2026
@Kenny-Vilella @eric-heiden
Bugfix: Add default joint friction value in USD parser (newton-physic…
07c2e49 
…s#1018)

Co-authored-by: Eric Heiden <eheiden@nvidia.com>
@coderabbitai coderabbitai Bot mentioned this pull request Feb 18, 2026
Merged
3 tasks
@coderabbitai coderabbitai Bot mentioned this pull request Apr 6, 2026
Merged
mmacklin pushed a commit to mmacklin/newton that referenced this pull request Apr 7, 2026
@Kenny-Vilella @eric-heiden
Bugfix: Add default joint friction value in USD parser (newton-physic…
e997f7d 
…s#1018)

Co-authored-by: Eric Heiden <eheiden@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eric-heiden eric-heiden eric-heiden approved these changes

@Milad-Rakhsha-NV Milad-Rakhsha-NV Awaiting requested review from Milad-Rakhsha-NV

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Kenny-Vilella @eric-heiden