Add source-neutral USD schema resolution - #3568
Closed
adenzler-nvidia wants to merge 23 commits into
Closed
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an opt-in source-neutral USD schema resolution API, schema-owned fallback catalogs, source-aware resolver mappings, importer plumbing, documentation, and regression tests. Legacy fallback behavior is audited through migration warnings while composed resolution applies schema ownership and fallback precedence. ChangesUSD schema resolution
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ModelBuilder
participant parse_usd
participant SchemaResolverManager
participant SchemaResolution
participant SchemaRegistry
ModelBuilder->>parse_usd: pass schema_resolution
parse_usd->>SchemaResolverManager: configure composed resolution
SchemaResolverManager->>SchemaResolution: resolve logical attribute
SchemaResolution->>SchemaRegistry: query applied-schema fallback
SchemaResolverManager-->>parse_usd: return value and provenance
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
adenzler-nvidia
commented
Jul 20, 2026
| <!-- SPDX-License-Identifier: CC-BY-4.0 --> | ||
|
|
||
| # Source-neutral USD schema resolution | ||
|
|
Member
Author
There was a problem hiding this comment.
This file will be removed for the final version, but it motivates the design for now so I think it's useful to have it in the PR.
Member
There was a problem hiding this comment.
Can we just migrate this doc to an Issue that the PR implements?
Member
Author
There was a problem hiding this comment.
I'd like to keep it up-to-date while the PR evolves, once final I will move the design to the PR description.
3 tasks
❌ 5 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Add an opt-in composed resolver path that gives applied schemas ownership of their unauthored properties. Prefer registered PXR fallbacks and use a built-in catalog when vendor schema plugins are unavailable, while keeping the existing resolver behavior unchanged by default.
3 tasks
Preserve existing add_usd resolver results while auditing where applied USD schema fallbacks will change precedence. Emit one migration warning per import and leave the source-neutral facade to the follow-up change.
Route legacy auditing and composed resolution through one private policy. Keep specialized joint paths on that policy so the future fallback switch requires no public API or scattered importer edits.
Reuse authored reads while auditing future schema fallbacks, and keep PXR-only custom getters on the legacy path when they cannot evaluate a source-neutral fallback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@CHANGELOG.md`:
- Line 12: Update the Changed entry in CHANGELOG.md to include migration
guidance: instruct users to explicitly author the intended values for affected
USD schema properties before the future fallback change, preserving the
resolver’s current behavior.
🪄 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: 737c985a-30bd-4256-9a6b-6be88783f3a8
📒 Files selected for processing (11)
CHANGELOG.mddocs/api/newton_usd.rstdocs/concepts/usd_parsing.rstdocs/design/schema-resolution.mddocs/index.rstnewton/_src/sim/builder.pynewton/_src/usd/schema_resolver.pynewton/_src/utils/import_usd.pynewton/tests/test_import_usd.pynewton/tests/test_schema_resolver.pynewton/usd.py
🚧 Files skipped from review as they are similar to previous changes (4)
- newton/_src/sim/builder.py
- docs/design/schema-resolution.md
- newton/tests/test_schema_resolver.py
- newton/_src/utils/import_usd.py
Distinguish expected future-resolution gaps from resolver failures so compatibility auditing cannot hide callback bugs. Treat registered properties without raw fallbacks as missing and report unauditable legacy values explicitly.\n\nMake first-party partial-schema and external-asset coverage assert the exact migration warning while preserving strict handling for every unexpected warning.
Let USD consumers adopt applied-schema fallback precedence during the compatibility period. Keep legacy resolution as the default while making its deprecation actionable, and migrate the Unitree examples to exercise the target behavior.
Distinguish the release default from explicit composed and legacy choices. This keeps the later default flip internal while allowing consumers to pin either behavior during migration.
Keep the public migration switch as a conventional false-by-default boolean. Consumers can opt into applied-schema fallback precedence explicitly while existing imports retain their current behavior.
Store the migration choice as the public boolean instead of a second policy enum, and declare the source-neutral reader callback on the resolver descriptor. Reuse authored reads in specialized joint auditing, share velocity-limit handling, and attribute migration warnings to consumer code.
Preserve the builder margin when PhysX's offset fallback selects its engine default, avoiding invalid shape margins. Match the built-in joint velocity fallback to the current PhysX schema when its plugin is unavailable.
Normalize legacy and composed fallback values with the same importer rules before deciding whether migration guidance is needed. Keep warnings for effective changes while allowing equivalent upgrades to run cleanly.
Adopt the shared joint DOF resolver added on main while preserving migration-aware velocity and limit fallback handling for ordinary, merged, and D6 joints.
Remove explicit negative warning assertions from schema fallback tests. The suite-wide strict warning policy now detects regressions while positive assertions remain for warnings that are required behavior.
Rely on USD schema definitions for authoritative fallbacks instead of maintaining a copied catalog. Keep unregistered resolvers functional through their compatibility defaults after importer defaults. Exercise the composed behavior in functional tests and reserve migration warning assertions for focused compatibility coverage.
Introduce an opt-in resolution facade that shares schema ownership, fallbacks, priority, and transformations between PXR and populated scene consumers. Preserve the legacy resolver path while cataloging built-in Newton, PhysX, and MuJoCo schema defaults.
Document the experimental composed-resolution API in the changelog and user guide. Reject unknown logical keys and tailor missing-fallback errors to source-neutral consumers so integration mistakes fail clearly.
Add a regression for per-axis PhysX D6 limit gain lookup and record the user-visible correction in the changelog.
Describe non-PXR consumers without tying the resolver contract or rollout to a specific downstream integration.
Replace the opaque factory with one typed SchemaResolution object used by both PXR and mapping sources. Route schema_resolvers through the same private compatibility policy so setup does not change import semantics.
Treat a key present in the source-neutral defaults mapping as an explicit importer choice even when its value is None. This lets importers defer to their builder defaults without falling through to compatibility defaults stored on resolver definitions.
Let source-neutral adapters supply registered schema metadata at resolution time while keeping the PXR registry authoritative for USD imports. Remove the copied fallback catalog so unregistered schemas retain only their compatibility behavior.
adenzler-nvidia
force-pushed
the
adenzler/schema-resolver-design
branch
from
July 23, 2026 16:11
7a84969 to
5baabff
Compare
3 tasks
3 tasks
3 tasks
Member
|
Superseded by #3984 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds one typed, source-neutral
SchemaResolutionobject shared byModelBuilder.add_usd()and consumers of populated or mapping-based scene data. Callers configure resolver priority and the fallback migration choice once, then pass the object toadd_usd(..., schema_resolution=resolution)or callresolution.resolve(...)directly. Both paths use the same schema ownership, USD fallback catalog, conversions, and boolean compatibility choice.The existing experimental
schema_resolvers=argument remains as a compatibility shorthand: internally it constructs the sameSchemaResolution. Supplying both arguments is rejected.use_applied_schema_fallbacks=Falseis the compatibility default; setting it toTrueonSchemaResolutionselects composed applied-schema fallbacks for both PXR and non-PXR sources without migration warnings. The directadd_usd()boolean is shorthand when no shared resolution object is supplied.The PXR adapter supplies registered schema fallbacks when available. Source-neutral consumers use the built-in catalog or explicit catalog additions, but do not need to construct PXR prims. Resolver definitions and evaluation internals remain private behind the small public object.
This first step deliberately leaves topology discovery, batching, Warp lowering, and direct builder-buffer population out of scope. Those can replace the scalar source adapter later without changing resolver configuration.
PR relationship and dependencies
main.Checklist
CHANGELOG.mdhas been updatedAPI
The design rationale and migration/removal sequence are documented in
docs/design/schema-resolution.md.