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.

Sign up for GitHub

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

Jump to bottom

[Bug Report] Inaccurate 'is_success' Condition Testing in lift_env.py #86

Closed
2 tasks done
yangcyself opened this issue May 23, 2023 · 1 comment
Closed
2 tasks done

[Bug Report] Inaccurate 'is_success' Condition Testing in lift_env.py #86

yangcyself opened this issue May 23, 2023 · 1 comment
Assignees
@yangcyself

Comments

@yangcyself
Copy link
Contributor

Describe the bug

In the line , the code checks if the object_position_error is less than 0.02 to determine if the task is a success. However, if the object_position_error is not less than 0.02, the task can still be marked as a success if reset_buf is set to 1 (which might happen due to the time limit being reached).

self.extras["is_success"] = torch.where(object_position_error < 0.02, 1, self.reset_buf)

To ensure that the task is only marked as a success when the object_position_error is less than 0.02, I propose changing the above line to:

self.extras["is_success"] = torch.where(object_position_error < 0.02, 1, 0)

In this way, the success condition will solely depend on the object_position_error, which seems to be the intended behavior.

Steps to reproduce

Additional context

Checklist

  • I have checked that there is no similar issue in the repo (required)
  • I have checked that the issue is not in running Isaac Sim itself and is related to the repo
@Mayankm96
Copy link
Contributor

Can you send the PR with this fix and check the training still works? Thanks!

@yangcyself yangcyself mentioned this issue May 25, 2023
Closed
5 tasks
Mayankm96 added a commit that referenced this issue Jul 24, 2023
@Mayankm96
Fixes utils operations for math and string (#86)
f4bb987 
# Description

* Fixes the `omni.isaac.orbit.utils.math.quat_apply_yaw` to compute the
yaw quaternion correctly.
* Adds functions to convert string and callable objects in the
`omni.isaac.orbit.utils.string` module. The function can deal with both
module functions and lambda expressions.

## Type of change

- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)

## 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 updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
Mayankm96 added a commit that referenced this issue Dec 22, 2023
@Mayankm96
Fixes utils operations for math and string (#86)
b4ccade 
# Description

* Fixes the `omni.isaac.orbit.utils.math.quat_apply_yaw` to compute the
yaw quaternion correctly.
* Adds functions to convert string and callable objects in the
`omni.isaac.orbit.utils.string` module. The function can deal with both
module functions and lambda expressions.

## Type of change

- Bug fix (non-breaking change which fixes an issue)
- New feature (non-breaking change which adds functionality)

## 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 updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
kellyguo11 pushed a commit to kellyguo11/IsaacLab-public that referenced this issue Jul 12, 2024
@nv-apoddubny
Changing base docker image to internal one (isaac-sim#86)
d045df2 
# Description

Changing base docker image to the internal one
[nvcr.io/nvidian/isaac-sim](http://nvcr.io/nvidian/isaac-sim)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yangcyself yangcyself

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix the inaccurate "is_success" condition yangcyself/Orbit
2 participants
@Mayankm96 @yangcyself