Skip to content

Conversation

@syundo0730
Copy link
Contributor

@syundo0730 syundo0730 commented Feb 16, 2025

Description

Fix tensor size mismatch in reset_root_state_from_terrain

When reset_root_state_from_terrain is called from an EventTerm, we cannot predict which env_ids will be targeted. To prevent program crashes due to tensor size mismatches, this MR modifies the velocity calculation to use only the specified env_ids instead of referencing all environments.

Changes:

  • Updated velocity calculation to use env_ids indexing for default_root_state
  • This ensures tensor dimensions match between default_root_state and rand_samples

Fixes #1882

Type of change

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

Screenshots

NONE

Checklist

  • 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 updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@Mayankm96 Mayankm96 changed the title Fix crash when using reset_root_state_from_terrain Fixes setting of root velocities in the event term reset_root_state_from_terrain Feb 20, 2025
@Mayankm96 Mayankm96 added the bug Something isn't working label Feb 20, 2025
Copy link
Contributor

@Mayankm96 Mayankm96 left a comment

Choose a reason for hiding this comment

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

Minor comment. Thanks a lot for the fix!

Please rebase to main. There seem to be some merge conflicts.

syundo0730 and others added 4 commits February 21, 2025 10:42
reset_root_state_from_terraion
Co-authored-by: Mayank Mittal <12863862+Mayankm96@users.noreply.github.com>
Signed-off-by: Shundo Kishi <syundo0730@gmail.com>
@syundo0730 syundo0730 force-pushed the fix/reset-root-state-from-terrain branch from f6ebd49 to a080b48 Compare February 21, 2025 01:43
@syundo0730
Copy link
Contributor Author

Thanks so much for reviewing this!
I’ve updated the code based on your suggestions.
I’d appreciate it if you could take another look when you get a chance.

Thanks again!

@syundo0730 syundo0730 requested a review from Mayankm96 February 21, 2025 01:53
@Mayankm96 Mayankm96 merged commit f1a4975 into isaac-sim:main Feb 24, 2025
5 checks passed
jtigue-bdai pushed a commit that referenced this pull request Apr 14, 2025
…from_terrain` (#1884)

# Description

When reset_root_state_from_terrain is called from an EventTerm, we
cannot predict which env_ids will be targeted. To prevent program
crashes due to tensor size mismatches, this MR modifies the velocity
calculation to use only the specified env_ids instead of referencing all
environments.

Changes:
- Updated velocity calculation to use env_ids indexing for
default_root_state
- This ensures tensor dimensions match between default_root_state and
rand_samples

Fixes #1882 

## Type of change

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

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
ToxicNS pushed a commit to ToxicNS/IsaacLab that referenced this pull request Apr 24, 2025
…from_terrain` (isaac-sim#1884)

# Description

When reset_root_state_from_terrain is called from an EventTerm, we
cannot predict which env_ids will be targeted. To prevent program
crashes due to tensor size mismatches, this MR modifies the velocity
calculation to use only the specified env_ids instead of referencing all
environments.

Changes:
- Updated velocity calculation to use env_ids indexing for
default_root_state
- This ensures tensor dimensions match between default_root_state and
rand_samples

Fixes isaac-sim#1882 

## Type of change

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

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug Report] reset_root_state_from_terrain fails

2 participants