Skip to content

Restore Franka cloth motion-vector regression coverage - #7195

Merged
nvsekkin merged 10 commits into
isaac-sim:developfrom
nvsekkin:esekkin/cloth-motion-vectors
Aug 25, 2026
Merged

Restore Franka cloth motion-vector regression coverage#7195
nvsekkin merged 10 commits into
isaac-sim:developfrom
nvsekkin:esekkin/cloth-motion-vectors

Conversation

@nvsekkin

@nvsekkin nvsekkin commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Description

Restore meaningful Franka cloth motion-vector regression coverage after #6998 changed the task to clamp the cloth between supports.

  • Raise the cloth by 1 cm for the Newton + OVRTX motion-vector case so it undergoes visible motion.
  • Preserve the OVRTX third-frame workaround tracked by NVBug 6565960.
  • Re-enable Newton + OVRTX cloth motion-vector coverage and refresh its golden.
  • Scope the controlled fall and extra frame strictly to Newton + OVRTX, leaving all other backend/renderer combinations unchanged.
  • Keep Newton + Isaac RTX quarantined under its existing nondeterminism policy.

The third-frame workaround can be removed after the OVRTX fix is available in a post-GA release.

Type of change

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

Screenshots

Updated Newton + OVRTX motion-vector golden:
Updated Franka cloth motion-vector golden

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump extension.toml — CI handles that)
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

Release backport

  • Backport this pull request to the active release branch after it merges into develop

Pin CI to alpha.58 so renderer validation uses the current Kit build.
Start the cloth above its supports, preserve the OVRTX history workaround, and require real cloth motion before golden comparison.
@nvsekkin
nvsekkin requested a review from a team August 19, 2026 23:16
@github-actions github-actions Bot added isaac-lab Related to Isaac Lab team infrastructure labels Aug 19, 2026
Comment thread source/isaaclab_tasks/test/rendering_test_utils.py Outdated
Keep the workflow-managed latest-develop tag while pinning the validated image digest.
) -> None:
_skip_if_newton_motion_vectors(physics_backend, data_type)
if renderer != "ovrtx_renderer":
_skip_if_newton_motion_vectors(physics_backend, data_type)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

the motion vector for deformable is still observed missing in the isaac rtx path

@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Restores Franka cloth motion-vector regression coverage for the Newton and OVRTX combination.

  • Re-enables the previously skipped parameter combination.
  • Raises the cloth and advances one extra frame for the affected rendering case.
  • Refreshes the corresponding golden image and parametrization assertions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the previously reported image-tag validation issue is fixed because the current tag uses the required latest-develop@sha256: format.

Important Files Changed

Filename Overview
source/isaaclab_tasks/test/rendering_test_utils.py Scopes the controlled cloth fall, extra simulation frame, and skip removal to Newton with OVRTX motion vectors.
source/isaaclab_tasks/test/test_parametrization_helpers.py Updates expected marks for supported Franka OVRTX motion-vector parameters.
source/isaaclab_tasks/test/golden_images/franka_cloth/newton-ovrtx_renderer-motion_vectors.png Refreshes the Newton and OVRTX motion-vector regression golden.

Reviews (4): Last reviewed commit: "Merge remote-tracking branch 'upstream/d..." | Re-trigger Greptile

Comment thread .github/workflows/config.yaml Outdated
Let the workflow-managed image tag and pinned digest remain the source of truth.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

new golden shows the motion vector in frame 3 - will be addressed in ovrtx 0.5.0

Keep the rendering regression focused on the captured motion-vector golden instead of Newton displacement magnitude.
Comment thread .github/workflows/config.yaml
@nvsekkin
nvsekkin requested a review from mmichelis August 19, 2026 23:23

@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 PR restores Franka cloth motion-vector coverage, but the new pose offset and extra simulation step affect all enabled motion-vector backend/renderer combinations while only the Newton+OVRTX golden is updated.

  • Design and architecture: The workaround is implemented in shared Franka cloth rendering-test helpers and keyed only on data_type, so it changes existing motion-vector scenarios beyond the Newton+OVRTX combination being restored.
  • API: No public APIs or documented contracts are changed; the affected surface is limited to private rendering-test configuration, CI configuration, and golden test data.
  • Implementation: The 1 cm initial-position offset and second env.step apply to non-Newton motion-vector tests that still reach this path. Scope these changes to the intended backend/renderer combination or refresh every affected motion-vector golden.

Minor fixes needed. Posted 1 actionable finding inline.

Automated review; human maintainers own approval decisions.

Comment thread source/isaaclab_tasks/test/rendering_test_utils.py Outdated
@nvsekkin

Copy link
Copy Markdown
Collaborator Author

@greptile review

@AntoineRichard AntoineRichard left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is OVRTX 0.5 planned for GA?

@nvsekkin

Copy link
Copy Markdown
Collaborator Author

Is OVRTX 0.5 planned for GA?

no that would come after GA

@mmichelis mmichelis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the changes! Just one question on if the changes are scoped correctly, otherwise looks good to me.

Comment thread source/isaaclab_tasks/test/rendering_test_utils.py Outdated
Limit the controlled fall and history workaround to validated Newton OVRTX coverage, and drop the unrelated Isaac Sim image bump.
@nvsekkin

Copy link
Copy Markdown
Collaborator Author

@greptile review

Resolve grouped rendering parameters while preserving scoped Newton OVRTX cloth motion coverage.
@nvsekkin

Copy link
Copy Markdown
Collaborator Author

@greptile review

@nvsekkin
nvsekkin enabled auto-merge (squash) August 24, 2026 23:10
@nvsekkin
nvsekkin merged commit 553efa6 into isaac-sim:develop Aug 25, 2026
48 of 49 checks passed
@isaaclab-bot

isaaclab-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Backported to release/3.0.0 as d1ab276.

isaaclab-bot Bot pushed a commit that referenced this pull request Aug 25, 2026
# Description

Restore meaningful Franka cloth motion-vector regression coverage after
#6998 changed the task to clamp the cloth between supports.

- Raise the cloth by 1 cm for the Newton + OVRTX motion-vector case so
it undergoes visible motion.
- Preserve the OVRTX third-frame workaround tracked by NVBug 6565960.
- Re-enable Newton + OVRTX cloth motion-vector coverage and refresh its
golden.
- Scope the controlled fall and extra frame strictly to Newton + OVRTX,
leaving all other backend/renderer combinations unchanged.
- Keep Newton + Isaac RTX quarantined under its existing nondeterminism
policy.

The third-frame workaround can be removed after the OVRTX fix is
available in a post-GA release.

## Type of change

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

## Screenshots
Updated Newton + OVRTX motion-vector golden:
![Updated Franka cloth motion-vector
golden](https://github.com/nvsekkin/IsaacLab/blob/esekkin/cloth-motion-vectors/source/isaaclab_tasks/test/golden_images/franka_cloth/newton-ovrtx_renderer-motion_vectors.png?raw=true)


## Checklist

- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have added a changelog fragment under
`source/<pkg>/changelog.d/` for every touched package (do **not** edit
`CHANGELOG.rst` or bump `extension.toml` — CI handles that)
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

## Release backport

- [x] <!-- backport-active-release --> Backport this pull request to the
active release branch after it merges into `develop`

(cherry picked from commit 553efa6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants