Skip to content

Fix Factory collision prim resolution - #7093

Merged
ooctipus merged 1 commit into
isaac-sim:developfrom
ooctipus:zhengyuz/fix-factory-env-regex
Aug 14, 2026
Merged

Fix Factory collision prim resolution#7093
ooctipus merged 1 commit into
isaac-sim:developfrom
ooctipus:zhengyuz/fix-factory-env-regex

Conversation

@ooctipus

Copy link
Copy Markdown
Collaborator

Description

Fixes IsaacContrib-Factory-Franka startup after segment-safe prim-path expressions were introduced in #6841.

The Factory collision analyzer assumed cloned environment expressions contained .*. It rewrote that spelling to 0 to find a source prim, then rebuilt an env_.* expression. The current {ENV_REGEX_NS} expansion is /World/envs/env_[^/]+, so the rewrite no longer produced a concrete USD path and startup failed with:

ValueError: Prim at path /World/envs/env_[^/]+/Robot is not valid.

This change resolves each collision body through resolve_matching_prims_from_source, the clone-plan-owned resolver. Its returned destination path expression is passed directly to point-cloud sampling, removing both wildcard-spelling rewrites and keeping ownership of clone-path resolution in the cloning subsystem.

No new dependencies or test files are introduced.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Screenshots

Not applicable; this is an environment-startup fix.

Validation

  • Reproduced the exact failure on unmodified latest develop with the existing consolidated Factory smoke case.
  • Existing IsaacContrib-Factory-Franka smoke case: 1 passed in 39.33s, including two environments and 20 random-action steps.
  • Full uv run isaaclab -f equivalent repository hook suite: passed.
  • git diff --check upstream/develop...HEAD: passed.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with isaaclab -f
  • Documentation changes are not required; the changelog fragment documents the fix
  • My changes generate no new warnings
  • The existing consolidated Factory smoke test proves the fix; no new test file is required
  • I have added a changelog fragment under source/isaaclab_tasks/changelog.d/
  • My name already exists in CONTRIBUTORS.md

Resolve collision analyzer bodies through the clone plan instead of rewriting environment wildcard spellings. This keeps Factory compatible with segment-safe prim-path expressions.
@ooctipus
ooctipus requested a review from a team August 14, 2026 05:15
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Aug 14, 2026

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isaac Lab Review Bot

The change replaces Factory collision analyzer wildcard string rewrites with the cloning subsystem’s prim resolver and adds the required task-package changelog fragment. The changed path-resolution flow is consistent with segment-safe environment expressions.

  • Design and architecture: Clone-path resolution is now owned by resolve_matching_prims_from_source rather than duplicated through assumptions about .*, env_0, or env_.*. This is a focused correction that preserves the surrounding collision-analysis design.
  • API: No public signature or consumer interface changes are introduced. CollisionAnalyzer still passes a string prim-path pattern to sample_object_point_cloud, now using the resolver-provided destination expression. The changelog fragment is correctly scoped to isaaclab_tasks.
  • Implementation: The per-body predicate resolves one rigid-body match and forwards its destination pattern directly to point-cloud sampling. Existing point-cloud filtering and body-index bookkeeping remain unchanged. A missing or ambiguous body now surfaces through the resolver’s match validation, which is a reasonable diagnostic tradeoff.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Replaces Factory collision prim-path string rewriting with the clone-plan-aware resolver, allowing segment-safe environment expressions to initialize correctly.

  • Resolves each configured rigid body through resolve_matching_prims_from_source.
  • Passes the resolver-owned destination expression directly into point-cloud sampling.
  • Adds a changelog fragment documenting the Factory startup fix.

Confidence Score: 5/5

The PR appears safe to merge with no actionable regressions identified.

The resolver returns the canonical segment-safe destination expression accepted by clone-path matching and point-cloud source discovery, while removing the brittle wildcard spelling assumptions that caused Factory startup to fail.

Important Files Changed

Filename Overview
source/isaaclab_tasks/isaaclab_tasks/contrib/nist/utils/collision_analyzer.py Replaces wildcard-specific path manipulation with the canonical clone-plan resolver; the returned segment-safe expression is compatible with downstream source discovery.
source/isaaclab_tasks/changelog.d/zhengyuz-factory-env-regex.rst Accurately documents the Factory startup fix.

Reviews (1): Last reviewed commit: "Fix Factory collision prim resolution" | Re-trigger Greptile

@ooctipus
ooctipus merged commit 5d254cc into isaac-sim:develop Aug 14, 2026
73 of 75 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants