habitat-lab icon indicating copy to clipboard operation
habitat-lab copied to clipboard

[Bug Fix] Fixing rotational bug in `tasks.rearrange.utils.get_camera_transform`

Open zephirefaith opened this issue 11 months ago • 2 comments

Motivation and Context

Fixes issue #1804

How Has This Been Tested

Tested with zf/obs-wm-updates branch of habitat-llm. Following is the relevant driver code:

fixed_pose = get_camera_transform(
                    self.sim.agents_mgr._all_agent_data[0].articulated_agent,
                    camera_name=f"{curr_agent}_{camera_source}_rgb",
                )

Check fixed point-clouds generated through this method in this PR comment.

Types of changes

  • [Docs change] Addition or changes to the documentation
  • [Refactoring] Large changes to the code that improve its functionality or performance
  • [Dependency Upgrade] Upgrades one or several dependencies in habitat
  • [Bug Fix] (non-breaking change which fixes an issue)
  • [Development] A pull request that add new features to the habitat-lab task and environment codebase. Development Pull Requests must be small (less that 500 lines of code change), have unit testing, very extensive documentation and examples. These are typically new tasks, environments, sensors, etc... The review process for these Pull Request is longer because these changes will be maintained by our core team of developers, so make sure your changes are easy to understand!
  • [Experiment] A pull request that add new features to the habitat-baselines training codebase. Experiments Pull Requests can be any size, must have smoke/integration tests and be isolated from the rest of the code. Your code additions should not rely on other habitat-baselines code. This is to avoid dependencies between different parts of habitat-baselines. You must also include a README that will document how to use your new feature or trainer. You will be the maintainer of this code, if the code becomes stale or is not supported often enough, we will eventually remove it.

Checklist

  • [ ] My code follows the code style of this project.
  • [ ] I have updated the documentation if required.
  • [ ] I have read the CONTRIBUTING document.
  • [ ] I have completed my CLA (see CONTRIBUTING)
  • [ ] I have added tests to cover my changes if required.

zephirefaith avatar Mar 11 '24 16:03 zephirefaith

Based on offline conversations:

  • @aclegg3's concern that the current code uses value of (0, 0, 0) as an unassigned default while it is a valid setting. Should be replaced with None instead.
  • Let's wrap get_camera_from_link logic as a util and use it consistently in setter + getter in: habitat.tasks.rearrange.utils.get_camera_transform, kinematic_humanoid.update and manipulator.update
  • Add a print warning notifying change in logic and a use_deprecated_logic flag to make sure logic change does not break things for other users who depend on old logic (CC: @jimmytyyang )

zephirefaith avatar Mar 15 '24 15:03 zephirefaith

@jimmytyyang @xavierpuigf Please take a look at this change as it directly touches your agent functions.

@aclegg3 Pushed the changes we discussed, please take a look and leave comments. However, let's not merge this until Xavi has taken a look and approved. I already walked Jimmy through this and I think the code changes will support his use-case.

zephirefaith avatar Mar 19 '24 19:03 zephirefaith