Conversation
There was a problem hiding this comment.
Other comments (17)
-
tools/rosbag_writer.py (165-170)
The `write_pointcloud` method always uses `serialize_cdr()` regardless of the ROS version, while the `write` method correctly uses `serialize_ros1()` for ROS1 and `serialize_cdr()` for ROS2. This inconsistency could cause issues when writing pointcloud data to ROS1 bags.
def write_pointcloud(self, msg: sensor_msgs__msg__PointCloud2, topic_name: str, timestamp: int) -> None: if self.ros == "ROS1": self.writer.write( self.connection_dict[topic_name], timestamp, self.typestore.serialize_ros1(msg, self.pointcloud_msgtype), ) elif self.ros == "ROS2": self.writer.write( self.connection_dict[topic_name], timestamp, self.typestore.serialize_cdr(msg, self.pointcloud_msgtype), ) -
tools/simple_inference.py (110-127)
The docstring for the filter method doesn't match the actual parameters. It describes 'pred_dicts' as a list of dictionaries, but the method takes a single 'pred_dict' parameter.
@staticmethod def filter(pred_dict: dict[str, torch.Tensor], score_threshold: float = 0.0) -> dict[str, torch.Tensor]: """Filter boxes with score threshold. Parameters ---------- pred_dict : dict[str, torch.Tensor] Prediction results. The dict contains: - pred_boxes: (M, 9) array in (x, y, z, dx, dy, dz, yaw, pitch, roll) format. - pred_scores: (M,) array. Score of each box. - pred_labels: (M,) array. Label of each box. score_threshold : float Score threshold. Returns ------- filtered_pred_dict : dict[str, torch.Tensor] Filtered prediction results. """ - tools/demo.py (57-57) There's a change in the expected format of .bin files from 4 columns to 5 columns. If existing .bin files have only 4 columns, this will cause shape mismatch errors. Consider adding a check for the point cloud dimensions or ensuring backward compatibility.
- tools/demo.py (39-46) When `root_path` is not a directory and its suffix is not in `AVAILABLE_EXTENSIONS`, `data_file_list` will be empty. This could lead to unexpected behavior. Consider adding an error message or handling for this case.
-
tools/visual_utils/open3d_vis_utils.py (38-38)
There's a potential error on line 38 where ref_boxes is filtered using ref_scores without checking if ref_scores is None. This will cause a runtime error if ref_boxes is provided but ref_scores is not.
if ref_boxes is not None and ref_scores is not None: ref_boxes = ref_boxes[ref_scores.cpu().numpy() > threshold] - requirements.txt (90-90) I notice that tensorflow and tensorflow-estimator dependencies were removed, along with waymo-open-dataset-tf-2-5-0. If the LION model still needs to process Waymo datasets, this might cause runtime errors. Please confirm that the Waymo dataset functionality is no longer needed or that it's handled by another dependency.
- requirements.txt (107-109) The newly added dependencies (open3d, av2, pypcd4) don't have version specifications. To ensure reproducible builds and avoid compatibility issues in the future, consider pinning specific versions for these packages.
- tools/output_manager.py (23-23) The timestamp conversion (`timestamp / 10**9`) assumes the input timestamp is in nanoseconds, but this isn't documented. Consider adding a docstring to clarify the expected timestamp format or make the conversion more explicit with a named variable.
- tools/pointcloud_loader.py (54-54) Consider catching specific exceptions instead of a generic `Exception` when trying to open a file as a rosbag. This will prevent masking unexpected errors that should be propagated.
- tools/simple_inference.py (174-177) The verify_args function only checks if the output_dir suffix is '.bag' for ROS1 format, but not for ROS2 format. Consider adding a similar check for ROS2 if it also requires a specific file extension.
- pcdet/datasets/nuscenes/nuscenes_dataset.py (315-315) Using a hardcoded absolute path ('/mnt/dlbox/') will make the code less portable. Consider using a configurable path (via environment variables or config files) or keeping the relative path approach to ensure the code works across different environments.
- tools/cfgs/dataset_configs/nuscenes_dataset.yaml (2-2) The DATA_PATH has been changed to an absolute path (`/mnt/dlbox/public_data/nuscenes/`) which might be specific to your local environment. Consider using a relative path or an environment variable to make the configuration more portable across different environments.
- tools/output_manager.py (34-43) The `visualize_results` method uses the imported visualization module `V` which is marked with `# noqa: F401`, suggesting it might not always be used. Consider adding error handling in case the visualization library is not available or properly initialized.
-
pyproject.toml (16-16)
This file is missing a newline at the end. It's a good practice to ensure all text files end with a newline character to comply with POSIX standards and avoid potential issues with some tools.
docstring-code-line-length = 60 -
tools/autoware_perception_msgs/msg/TrafficLightElement.msg (15-16)
There's inconsistent spacing in the arrow constant definitions. For better code consistency, consider standardizing the spacing around equals signs.
uint8 UP_LEFT_ARROW = 5 uint8 UP_RIGHT_ARROW = 6 -
tools/mapping.json (21-24)
This file is missing a newline at the end of the file. Adding a newline at the end of text files is a common best practice that helps with concatenation and prevents issues with some tools.
"7": [ "pedestrian" ] } -
tools/simple_inference.py (128-136)
The variable `filtered_pred_dict` is initialized but then immediately reassigned, making the first assignment unnecessary.
scores = pred_dict["pred_scores"].cpu().numpy() mask = scores >= score_threshold filtered_pred_dict = { "pred_boxes": pred_dict["pred_boxes"][mask], "pred_scores": pred_dict["pred_scores"][mask], "pred_labels": pred_dict["pred_labels"][mask], }
💡 To request another review, post a new comment with "/windsurf-review".
| # Create DataFrame and save to CSV | ||
| csv_result = np.hstack((pred_boxes, pred_scores[:, None])) | ||
| df = pd.DataFrame(csv_result, columns=["x", "y", "z", "l", "w", "h", "yaw", "pitch", "roll", "score"]) | ||
| df.insert(0, "timestamp", np.full((len(pred_labels), 1), timestamp / 10**9)) |
There was a problem hiding this comment.
The timestamp column is created with a 2D array shape (len(pred_labels), 1), but DataFrame columns should be 1D. This might cause unexpected behavior. Consider using:
| df.insert(0, "timestamp", np.full((len(pred_labels), 1), timestamp / 10**9)) | |
| df.insert(0, "timestamp", np.full(len(pred_labels), timestamp / 10**9)) |
tools/pointcloud_loader.py
Outdated
| else: | ||
| self.data_size = len(self.pointcloud_path_list) | ||
|
|
||
| def get_pointcloud(self) -> Generator[list[np.ndarray, str, float], None, None]: |
There was a problem hiding this comment.
The return type annotation for get_pointcloud() is incorrect. It should be Generator[tuple[np.ndarray, Union[int, float]], None, None] instead of Generator[list[np.ndarray, str, float], None, None] to match what's actually being yielded.
| """Load point cloud from file. | ||
|
|
||
| Returns | ||
| ------- | ||
| points : np.ndarray | ||
| (N, 4) array. Each point is represented by (x, y, z, intensity). | ||
| """ |
There was a problem hiding this comment.
The docstring for get_pointcloud() doesn't match the actual return value. It should document that the method yields tuples of (points, counter/timestamp) rather than just points.
入力として、
bin,npy,pcd,rosbag、出力として、csv、rosbagが使える推論apiです。