-
Notifications
You must be signed in to change notification settings - Fork 8
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
Lidar Scene Transforms #265
base: master
Are you sure you want to change the base?
Changes from all commits
c284792
9b31d95
84e1eec
5f155ec
19c57c6
479c342
96cacb9
4095a57
3cea689
26c3578
f3c2fa7
9d9e32d
5021ce6
4cca2e6
f7c0d39
203ec13
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
from nucleus.sensorfusion import * | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is close to being a real unit test but A) The test data you use isn't being checked in Since this is a first pass, fine to skip for now, just letting you know this is VERY close! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok wait I changed my mind, we need to turn this into a unit test and at least check that it runs. Ok if you don't know how to add checks for the expected results, but somewhere else in the code review I saw we are missing a dependency. If we make this a real unit test, this would have caught that issue. For an example of a unit test, see any of the files under test/ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will check this out! |
||
import numpy as np | ||
import glob | ||
|
||
def format_pointcloud(lidar_np): | ||
mask = lidar_np[:, 4] == 0.0 | ||
pc_1 = lidar_np[mask, :] | ||
pc_1 = np.delete(pc_1, (4), 1) | ||
return pc_1 | ||
|
||
npz_files = sorted(glob.glob("sdk-sample-data/*")) | ||
npz_files.remove("sdk-sample-data/extrinsics_seq000-003_11042021.npz") | ||
|
||
updated_extrinsics = np.load("sdk-sample-data/extrinsics_seq000-003_11042021.npz", allow_pickle = True) | ||
wnsl_extrinsics = updated_extrinsics['camera_WindshieldNarrowStereoLeft_lidar_extrinsic'] | ||
print(f"Camera Extrinsics: \n{wnsl_extrinsics}") | ||
|
||
cam_1_calib = CameraCalibration(extrinsic_matrix=wnsl_extrinsics) | ||
print(f"Original Camera Pose:{cam_1_calib.pose}") | ||
|
||
RawScene = RawScene() | ||
for idx,npz_fp in enumerate(npz_files): | ||
print(f"Frame Index: {idx}") | ||
|
||
frame_npz = np.load(npz_fp, allow_pickle=True) | ||
|
||
pointcloud_np= format_pointcloud(frame_npz['points']) | ||
print(f"PointCloud Shape: {pointcloud_np.shape}") | ||
|
||
pointcloud = RawPointCloud(points=pointcloud_np) | ||
|
||
print(f"World Coordinate Transformation:\n{frame_npz['vehicle_local_tf']}") | ||
frame_pose = Transform(frame_npz['vehicle_local_tf']) | ||
print(f"Frame Pose: {frame_pose}") | ||
|
||
raw_frame = RawFrame(lidar=pointcloud, cam1=cam_1_calib, device_pose=frame_pose) | ||
RawScene.add_frame(raw_frame) | ||
|
||
print(f"Frame 5, Point1 PreTransform: {RawScene.frames[4].items['lidar'].points[0]}") | ||
print(f"Frame 5, Camera Extrinsics PreTransform: {RawScene.frames[4].items['cam1'].pose}") | ||
RawScene.apply_transforms(relative=True) | ||
print(f"Frame 5, Point1 in World: {RawScene.frames[4].items['lidar'].points[0]}") | ||
print(f"Frame 5, Camera Extrinsics PostTransform: {RawScene.frames[4].items['cam1'].pose}") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,162 @@ | ||
import numpy as np | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the ordering of these imports is off, we have a pre-commit hook that fixes import oordering automatically, my bet is that you just don't have it installed. Try running the following in your repo: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Having trouble running this |
||
from nucleus.transform import Transform | ||
import copy | ||
from typing import List | ||
from dataclasses import dataclass | ||
|
||
from sensorfusion_utils import read_pcd | ||
|
||
@dataclass | ||
class RawPointCloud: | ||
''' | ||
RawPointClouds are containers for raw point cloud data. This structure contains Point Clouds as (N,3) or (N,4) numpy arrays | ||
Point cloud data is assumed to be in ego coordinates. | ||
|
||
If the point cloud is in world coordinates, one can provide the inverse pose as the "transform" argument. If this argument | ||
is present, the point cloud will be transformed back into ego coordinates by: | ||
|
||
transform.apply(points) | ||
|
||
or in the extended implementation: | ||
|
||
points_4d = np.hstack([points[:, :3], np.ones((points.shape[0], 1))]) | ||
transformed_4d = points_4d.dot(self.matrix.T) | ||
return np.hstack([transformed_4d[:, :3], points[:, 3:]]) | ||
|
||
|
||
Args: | ||
points (np.array): Point cloud data represented as (N,3) or (N,4) numpy arrays | ||
transform (:class:`Transform`): If in World Coordinate, the transformation used to transform lidar points into ego coordinates | ||
patrick-scale marked this conversation as resolved.
Show resolved
Hide resolved
|
||
''' | ||
points: np.array | ||
|
||
def __init__(self, points: np.array = None, transform: Transform = None): | ||
self.points = points | ||
patrick-scale marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if points is not None and (len(self.points.shape) != 2 or self.points.shape[1] not in [3, 4]): | ||
raise Exception(f'numpy array has unexpected shape{self.points.shape}. Please convert to (N,3) or (N,4) numpy array where each row is [x,y,z] or [x,y,z,i]') | ||
|
||
if transform is not None: | ||
self.points = transform.apply(points) | ||
|
||
def load_pcd(self, filepath: str, transform: Transform = None): | ||
''' | ||
Takes raw pcd file and reads in as numpy array. | ||
Args: | ||
filepath (str): Local filepath to pcd file | ||
transform (:class:`Transform`): If in world, the transformation used to transform lidar points into ego coordinates | ||
''' | ||
points = read_pcd(filepath) | ||
self.points = points | ||
if transform is not None: | ||
self.points = transform.apply(points) | ||
|
||
|
||
class CameraCalibration: | ||
''' | ||
CameraCalibration solely holds the pose of the camera | ||
This CameraCalibration will inevitably be transformed by the device_pose | ||
Args: | ||
extrinsic_matrix (np.array): (4,4) extrinsic transformation matrix representing device_to_lidar | ||
''' | ||
|
||
def __init__(self, extrinsic_matrix=None): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :nit: try making this a dataclass for some syntactic sugar! |
||
self.extrinsic_matrix = extrinsic_matrix | ||
|
||
@property | ||
def extrinsic_matrix(self): | ||
"""Camera extrinsic | ||
:getter: Return camera's extrinsic matrix (pose.inverse[:3, :4]) | ||
:setter: pose = Transform(matrix).inverse | ||
:type: 3x4 matrix | ||
""" | ||
return self.pose.inverse[:3, :4] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. self.pose may be did you mean to initialize it in the constructor? |
||
|
||
@extrinsic_matrix.setter | ||
def extrinsic_matrix(self, matrix): | ||
''' | ||
Sets pose as inverse of extrinsic matrix | ||
''' | ||
self.pose = Transform(matrix).inverse | ||
|
||
|
||
class RawFrame: | ||
''' | ||
RawFrames are containers for point clouds, image extrinsics, and device pose. | ||
These objects most notably are leveraged to transform point clouds and image extrinsic matrices to the world coordinate frame. | ||
Args: | ||
device_pose (:class:`Transform`): World Coordinate transformation for frame | ||
**kwargs (Dict[str, :class:`RawPointCloud, :class: CameraCalibration`]): Mappings from sensor name | ||
to pointcloud and camera calibrations. Each frame of a lidar scene must contain exactly one | ||
pointcloud and any number of camera calibrations | ||
''' | ||
def __init__(self, device_pose: Transform = None, **kwargs): | ||
self.device_pose = device_pose | ||
self.items = {} | ||
for key, value in kwargs.items(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to name the variables you expect in the constructor explicitly instead of relying on kwargs, whenever possible There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed - but it's tough because there is a variable number of camera items that they may add to each frame dependent on their sensor suite |
||
if isinstance(value, CameraCalibration): | ||
self.items[key] = copy.copy(value) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why copy? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise this basically passes by reference. We want to generate a xFrames camera calibrations and this will just continually update the same variable otherwise |
||
else: | ||
self.items[key] = value | ||
|
||
def get_world_points(self): | ||
"""Return the list of points with the frame transformation applied | ||
:returns: List of points in world coordinates | ||
:rtype: np.array | ||
""" | ||
for item in self.items: | ||
if isinstance(self.items[item], RawPointCloud): | ||
return np.hstack( | ||
[ | ||
self.device_pose @ self.items[item][:, :3], | ||
self.items[item][:, 3:4], | ||
self.items[item][:, 4:5] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add validation on item to make sure item.shape[1] >= 5 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It actually just will skip this step if the shape is < 5 (add nothing) |
||
] | ||
) | ||
|
||
|
||
class RawScene: | ||
''' | ||
RawsScenes are containers for frames | ||
Args: | ||
frames (:class:`RawFrame`): Indexed sequential frame objects composing the scene | ||
''' | ||
|
||
def __init__(self, frames: List[RawFrame] = []): | ||
if frames is None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just init to empty list as suggested above |
||
self.frames = [] | ||
else: | ||
self.frames = frames | ||
|
||
self.transformed = False | ||
|
||
def add_frame(self, frame: RawFrame): | ||
self.frames.append(frame) | ||
|
||
def make_transforms_relative(self): | ||
"""Make all the frame transform relative to the first transform/frame. This will set the first transform to position (0,0,0) and heading (1,0,0,0)""" | ||
offset = self.frames[0].device_pose.inverse | ||
for frame in self.frames: | ||
frame.device_pose = offset @ frame.device_pose | ||
|
||
def apply_transforms(self, relative: bool = False): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happens if a user double applies make_transforms_relative? is it bad? if yes, we should prevent them from doing it twice (maybe add a boolean flag to represent if it's already been applied) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commited new changes to handle double transformation |
||
if self.transformed: | ||
raise Exception("apply_transforms was called more than once. Raw Scene has already been transformed!") | ||
|
||
if relative: | ||
self.make_transforms_relative() | ||
|
||
for frame in self.frames: | ||
for item in frame.items: | ||
if isinstance(frame.items[item], RawPointCloud): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe can get rid of this check, what other class would it be an instance of? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be a CameraCalibration object |
||
frame.items[item].points = np.hstack( | ||
[ | ||
frame.device_pose @ frame.items[item].points[:, :3], | ||
frame.items[item].points[:, 3:4], | ||
frame.items[item].points[:, 4:5], | ||
] | ||
) | ||
else: | ||
wct = frame.device_pose @ frame.items[item].pose | ||
frame.items[item].pose = wct | ||
self.transformed = True |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import open3d as o3d | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be added as a project dependency using
I think it's as simple as
|
||
import numpy as np | ||
|
||
def read_pcd(pcd_filepath): | ||
''' | ||
Loads in pcd file and returns (N,3) or (N,4) numpy array | ||
|
||
Args: | ||
param pcd_filepath : filepath to local .pcd file | ||
type pcd_filepath: str | ||
|
||
Returns: | ||
point_numpy: (N,4) or (N,3) numpy array of points | ||
type point_numpy: np.array | ||
''' | ||
point_cloud = o3d.io.read_point_cloud(pcd_filepath) | ||
|
||
point_numpy = np.asarray(point_cloud.points)[:, :4] | ||
return point_numpy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do this elsewhere in the code but it's generally not good style, so good to avoid if possible