Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug Report] prim view order not correct in FrameTransformer #170

Closed
taochenshh opened this issue Dec 11, 2023 · 5 comments
Closed

[Bug Report] prim view order not correct in FrameTransformer #170

taochenshh opened this issue Dec 11, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@taochenshh
Copy link

Describe the bug

In this file, it says that the order of elements in first_env_prim_paths will be source frame followed by target frames. However, this is not guaranteed.

In my case,
body_names_regex is '/World/envs/env_.*/Robot/(fr3_link0|link_3_0_tip|link_15_0_tip|link_7_0_tip|link_11_0_tip)' where fr3_link0 is the source frame, but the first_env_prim_paths turns out to be:

['/World/envs/env_0/Robot/link_3_0_tip',
 '/World/envs/env_0/Robot/link_15_0_tip',
 '/World/envs/env_0/Robot/link_7_0_tip',
 '/World/envs/env_0/Robot/link_11_0_tip',
 '/World/envs/env_0/Robot/fr3_link0']

So the source frame is not the first element.

@Mayankm96 Mayankm96 added the bug Something isn't working label Dec 11, 2023
@Mayankm96
Copy link
Contributor

You are right. The ordering is determined by the PhysX parser and it does not necessarily need to be the order in which the regex expression is defined. We should keep them as two separate views to avoid confusion or re-index the starting frame.

@taochenshh
Copy link
Author

yes, the current code would lead to an error starting from this line as this line assumes the order is preseved.

@taochenshh
Copy link
Author

@Mayankm96 will this be fixed soon? One possible way to at least make sure the source frame is not included is replacing this line with something like:

target_frame_body_names = [x for x in first_env_body_names  if not re.match(self._tracked_body_names[0], x)]

@Mayankm96
Copy link
Contributor

Hi @taochenshh,

Our hands are tied right now since we are trying to get everything ready to merge devel to main. From the team's availability, we would very likely only be able to take a look after the winter holidays. If you can make a fix, that would be really useful. We can review it and add that patch fix.

@Mayankm96
Copy link
Contributor

Mayankm96 commented Jan 11, 2024

Sorry for the delay in fixing this bug report. It is now addressed in the commit: 620ce2b

Please re-open the issue if the problem still persists.

fatimaanes pushed a commit to fatimaanes/omniperf that referenced this issue Aug 8, 2024
# Description

The FrameTransformer sensor always assumed that the source frame index
is 0 in the parsed regex expression. However, a recent issue appeared
where this is not the case, and it led to a bug due to the hard-coded
value. This MR fixes the source frame index in the sensor and also adds
a unit test to check this behavior.

Fixes: isaac-sim#170

## 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
`./orbit.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 run all the tests with `./orbit.sh --test` and they pass
- [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

---------

Co-authored-by: Mayank Mittal <mittalma@leggedrobotics.com>
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

No branches or pull requests

2 participants