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

Add Camera Benchmark Tool and Allow Correct Unprojection of distance_to_camera depth image #976

Merged
merged 54 commits into from
Sep 25, 2024
Merged

Add Camera Benchmark Tool and Allow Correct Unprojection of distance_to_camera depth image #976

merged 54 commits into from
Sep 25, 2024

Conversation

glvov-bdai
Copy link
Collaborator

@glvov-bdai glvov-bdai commented Sep 11, 2024 •
edited
Loading

Description

This change allows a user to easily test different camera settings/configurations to see what works best for them.

This also includes a fix to be able to correctly unproject point clouds from the distance_to_camera replicator,
as the unproject_depth method assumes that data originates from distance_to_image_plane replicator and as such could previously result in distortion when previously used with the distance_to_camera replicator.
pyproject.toml

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Screenshots

saved_depth_images_timestep_7

saved_images_timestep_4

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

jsmith-bdai
jsmith-bdai approved these changes Sep 16, 2024
View reviewed changes
Copy link
Collaborator

@jsmith-bdai jsmith-bdai left a comment

Choose a reason for hiding this comment

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

LGTM with a few minor comments. Thanks for adding this useful tool 🎉

docs/source/how-to/estimate_how_many_cameras_can_run.rst Outdated Show resolved Hide resolved
source/extensions/omni.isaac.lab/docs/CHANGELOG.rst Outdated Show resolved Hide resolved
source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py Outdated Show resolved Hide resolved
@glvov-bdai glvov-bdai self-assigned this Sep 23, 2024
@jsmith-bdai
Copy link
Collaborator

@Mayankm96 @pascal-roth can you guys take another look, I think Gary has addressed most of the changes and this tool is quite useful so it would be good to get it merged to the repo!

jsmith-bdai
jsmith-bdai approved these changes Sep 23, 2024
View reviewed changes
Copy link
Collaborator

@jsmith-bdai jsmith-bdai left a comment

Choose a reason for hiding this comment

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

New changes LGTM, thanks again!

source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py Outdated Show resolved Hide resolved
source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py Outdated Show resolved Hide resolved
glvov-bdai and others added 3 commits September 24, 2024 10:00
@glvov-bdai @jsmith-bdai
Update source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py
17c74f3 
Co-authored-by: James Smith <142246516+jsmith-bdai@users.noreply.github.com>
Signed-off-by: glvov-bdai <glvov@theaiinstitute.com>
@glvov-bdai @jsmith-bdai
Update source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py
4f68566 
Co-authored-by: James Smith <142246516+jsmith-bdai@users.noreply.github.com>
Signed-off-by: glvov-bdai <glvov@theaiinstitute.com>
@glvov-bdai
Merge branch 'main' into feature/tiled_camera_examples
3e749e6
@glvov-bdai glvov-bdai mentioned this pull request Sep 24, 2024
Closed
Dhoeller19
Dhoeller19 approved these changes Sep 25, 2024
View reviewed changes
Copy link
Collaborator

@Dhoeller19 Dhoeller19 left a comment

Choose a reason for hiding this comment

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

This is cool stuff, thanks @glvov-bdai!

@glvov-bdai glvov-bdai mentioned this pull request Sep 25, 2024
Open
@glvov-bdai glvov-bdai dismissed stale reviews from Mayankm96 and pascal-roth September 25, 2024 19:13

No activity in a week, need to merge functionality for upstream manager based cartpole PR

@glvov-bdai
Copy link
Collaborator Author

glvov-bdai commented Sep 25, 2024 •
edited
Loading

@Dhoeller19 @kellyguo11 I don't have permission to merge but it's ready do you mind merging this in for me (or giving me the needed access to merge this)? EDIT: smitty got me

@jsmith-bdai jsmith-bdai merged commit f8d80cb into isaac-sim:main Sep 25, 2024
1 of 2 checks passed
Mayankm96
Mayankm96 reviewed Sep 25, 2024
View reviewed changes
source/extensions/omni.isaac.lab/docs/CHANGELOG.rst
@@ -2,6 +2,18 @@ Changelog
---------


0.24.14 (2024-09-20)
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: You need to update BOTH extension.toml and CHANGELOG.rst

Mayankm96
Mayankm96 reviewed Sep 25, 2024
View reviewed changes
source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py
@@ -988,7 +988,12 @@ def transform_points(

@torch.jit.script
def unproject_depth(depth: torch.Tensor, intrinsics: torch.Tensor) -> torch.Tensor:
r"""Unproject depth image into a pointcloud.
r"""Unproject depth image into a pointcloud. This method assumes that depth
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow google doc-style. The first line here is a one-line sumary. Everything else moves to a new para.

jsmith-bdai added a commit that referenced this pull request Sep 25, 2024
@glvov-bdai @garylvov @jsmith-bdai
Create Manager Based Cartpole Vision Example Environments (#995)
2804ff3 
# Description

Adds manager based cartpole vision example environments. 

Also uses the`convert_perspective_depth_to_orthogonal_depth `
functionality introduced in #976 , contains a duplicate copy of the
method and test for completeness of the PR. Will be synced with main to
remove this duplicate copy once #976 is merged into main, or #976 will
be synced with main if this PR is merged first

## Type of change

- New feature (non-breaking change which adds functionality)
- This change requires a documentation update

## Screenshots

![image](https://github.com/user-attachments/assets/4cce3923-b199-4469-b8d0-9d8d8abb3456)

## Checklist

- [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 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

## Testing

Training converged in both RGB and Depth at similar rates to the direct
environments

---------

Signed-off-by: garylvov <67614381+garylvov@users.noreply.github.com>
Signed-off-by: glvov-bdai <glvov@theaiinstitute.com>
Co-authored-by: garylvov <gary.lvov@gmail.com>
Co-authored-by: garylvov <67614381+garylvov@users.noreply.github.com>
Co-authored-by: James Smith <142246516+jsmith-bdai@users.noreply.github.com>
Mayankm96
Mayankm96 reviewed Sep 25, 2024
View reviewed changes
source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py
def convert_perspective_depth_to_orthogonal_depth(
perspective_depth: torch.Tensor, intrinsics: torch.Tensor
) -> torch.Tensor:
r"""Provided depth image(s) where depth is provided as the distance to the principal
Copy link
Contributor

Choose a reason for hiding this comment

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

r""" is only used when you have math equations. In all other cases, please resort to double ticks.

Mayankm96
Mayankm96 reviewed Sep 25, 2024
View reviewed changes
source/extensions/omni.isaac.lab/omni/isaac/lab/utils/math.py
The function assumes that the width and height are both greater than 1.
Args:
perspective_depth: The depth measurement obtained with the distance_to_camera replicator.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it is a math util, any mention of replicator does not make sense.

Mayankm96
Mayankm96 reviewed Sep 25, 2024
View reviewed changes
source/extensions/omni.isaac.lab/test/utils/test_math.py
@@ -376,6 +376,24 @@ def iter_old_quat_rotate_inverse(q: torch.Tensor, v: torch.Tensor) -> torch.Tens
iter_old_quat_rotate_inverse(q_rand, v_rand),
)

def test_depth_perspective_conversion(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please always add a docstring for a test so the description also is visible when you run the test.

Mayankm96
Mayankm96 reviewed Sep 25, 2024
View reviewed changes
source/standalone/tutorials/04_sensors/benchmark_cameras.py
#
# SPDX-License-Identifier: BSD-3-Clause

"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend moving this benchmarking to the benchmark folder instead of tutorials.

iamdrfly pushed a commit to iamdrfly/IsaacLab that referenced this pull request Nov 21, 2024
@glvov-bdai
Add Camera Benchmark Tool and Allow Correct Unprojection of distance_…
454a790 
…to_camera depth image (isaac-sim#976)
iamdrfly pushed a commit to iamdrfly/IsaacLab that referenced this pull request Nov 21, 2024
@glvov-bdai @garylvov @jsmith-bdai
Create Manager Based Cartpole Vision Example Environments (isaac-sim#995
ba7fdf6 
)

# Description

Adds manager based cartpole vision example environments. 

Also uses the`convert_perspective_depth_to_orthogonal_depth `
functionality introduced in isaac-sim#976 , contains a duplicate copy of the
method and test for completeness of the PR. Will be synced with main to
remove this duplicate copy once isaac-sim#976 is merged into main, or isaac-sim#976 will
be synced with main if this PR is merged first

## Type of change

- New feature (non-breaking change which adds functionality)
- This change requires a documentation update

## Screenshots

![image](https://github.com/user-attachments/assets/4cce3923-b199-4469-b8d0-9d8d8abb3456)

## Checklist

- [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 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

## Testing

Training converged in both RGB and Depth at similar rates to the direct
environments

---------

Signed-off-by: garylvov <67614381+garylvov@users.noreply.github.com>
Signed-off-by: glvov-bdai <glvov@theaiinstitute.com>
Co-authored-by: garylvov <gary.lvov@gmail.com>
Co-authored-by: garylvov <67614381+garylvov@users.noreply.github.com>
Co-authored-by: James Smith <142246516+jsmith-bdai@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mayankm96 Mayankm96 Mayankm96 left review comments

@jsmith-bdai jsmith-bdai jsmith-bdai approved these changes

@Dhoeller19 Dhoeller19 Dhoeller19 approved these changes

@kellyguo11 kellyguo11 Awaiting requested review from kellyguo11 kellyguo11 is a code owner

@pascal-roth pascal-roth Awaiting requested review from pascal-roth pascal-roth is a code owner

Assignees

@glvov-bdai glvov-bdai

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@glvov-bdai @Mayankm96 @jsmith-bdai @Dhoeller19 @pascal-roth @garylvov