feat: spatial temporal navigation #355
Conversation
|
@coderabbitai full review |
WalkthroughThe pull request introduces a new module for spatial-temporal navigation in the RAI (Robotics AI) application. The changes include creating a comprehensive data collection and memory retrieval system that integrates ROS2 message handling, image processing, vector database storage, and memory querying tools. The module enables collecting, describing, and retrieving spatial-temporal observations with support for position, timestamp, and text-based memory searches. Changes
Sequence DiagramsequenceDiagram
participant ROS2 as ROS2 System
participant DataCollection as Spatial Temporal Data Collection
participant VectorStore as Vector Store
participant MongoDB as MongoDB Collection
participant LLM as Language Model
ROS2->>DataCollection: Provide Transform and Image
DataCollection->>DataCollection: Build Observation
DataCollection->>LLM: Generate Description
LLM-->>DataCollection: Return Description
DataCollection->>VectorStore: Store Observation
DataCollection->>MongoDB: Store Observation Details
The sequence diagram illustrates the high-level workflow of the spatial-temporal data collection process, showing how data is collected from ROS2, processed, described, and stored in both a vector store and a MongoDB collection. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py (6)
16-23: Import Optimization: Duplicate Imports DetectedThe imports for
cv2andCvBridgeare redundant if they are not used extensively throughout the module. Ensure that all imported modules are necessary. Also, consider grouping related imports together and removing any unused ones to improve code readability and maintainability.
88-105: Simplifyros2_transform_stamped_to_positionFunctionThe function can be simplified by removing unnecessary comments and redundant code. Additionally, the use of
# type: ignorecomments suggests type-checking issues that should be addressed.Apply this diff to simplify the function:
-def ros2_transform_stamped_to_position( - transform_stamped: TransformStamped, -) -> PositionStamped: - return PositionStamped( - timestamp=transform_stamped.header.stamp.sec - + transform_stamped.header.stamp.nanosec / 1e9, - position=Pose( - x=transform_stamped.transform.translation.x, - y=transform_stamped.transform.translation.y, - z=transform_stamped.transform.translation.z, - ), - orientation=Orientation( - x=transform_stamped.transform.rotation.x, - y=transform_stamped.transform.rotation.y, - z=transform_stamped.transform.rotation.z, - w=transform_stamped.transform.rotation.w, - ), - ) +def ros2_transform_stamped_to_position(transform_stamped: TransformStamped) -> PositionStamped: + timestamp = transform_stamped.header.stamp.sec + transform_stamped.header.stamp.nanosec / 1e9 + position = Pose( + x=transform_stamped.transform.translation.x, + y=transform_stamped.transform.translation.y, + z=transform_stamped.transform.translation.z, + ) + orientation = Orientation( + x=transform_stamped.transform.rotation.x, + y=transform_stamped.transform.rotation.y, + z=transform_stamped.transform.rotation.z, + w=transform_stamped.transform.rotation.w, + ) + return PositionStamped(timestamp=timestamp, position=position, orientation=orientation)
108-119: Avoid ReinitializingCvBridgein Each CallThe
CvBridgeinstance is created every timeros2_image_to_imageis called. It would be more efficient to initializeCvBridgeonce and reuse it.[performance]
Apply this diff to initialize
CvBridgeglobally:+bridge = CvBridge() def ros2_image_to_image(ros2_image: Image) -> ImageStamped: logger.info("Converting ROS2 image to base64 image") - bridge = CvBridge() cv2_image = bridge.imgmsg_to_cv2(ros2_image) # Rest of the code...
152-157: Metadata Type Mismatch inVectorDatabaseEntryThe
metadatafield inVectorDatabaseEntryis typed asDict[str, str], but you might want to include other data types in the metadata, like timestamps or numerical IDs.Consider changing the metadata type:
-class VectorDatabaseEntry(BaseModel): - text: str - metadata: Dict[str, str] +class VectorDatabaseEntry(BaseModel): + text: str + metadata: Dict[str, Any]
160-177: Asynchronous Execution: Handle Database and Vector Store Operations ProperlyDatabase and vector store operations in
data_collection_pipelineare executed sequentially but might benefit from asynchronous execution to improve performance, especially when dealing with I/O-bound tasks.[performance]
Consider using asynchronous programming constructs like
asyncioor threading.
191-201: Resource Management: Node Shutdown inImageGrabberThe
ImageGrabbernode does not have a mechanism to gracefully shut down, which could lead to resource leaks when the application is terminated.Implement a shutdown method to properly clean up the node:
class ImageGrabber(Node): def __init__(self, image_topic: str): super().__init__("image_grabber") self.subscription = self.create_subscription( Image, image_topic, self.image_callback, 10 ) self.image: Image | None = None + def shutdown(self): + self.destroy_node()src/rai/rai/apps/__init__.py (1)
15-17: Maintain Consistency in Import StatementsIt's good practice to use absolute imports for clarity and to avoid potential issues with relative imports in complex packages.
Apply this diff to use absolute imports:
from .spatial_temporal_navigation.spatial_temporal_navigation import ( run_spatial_temporal_data_collection, )src/rai/rai/apps/spatial_temporal_navigation/tools.py (1)
104-121: Consistent Use of Input SchemasThe
GetMemoriesNearTimestampToolInputandGetMemoriesNearTextToolInputclasses are defined for input validation, butGetMemoriesNearPositionTooldirectly usesPoseas itsargs_schema. For consistency and clarity, consider defining a dedicated input schema for each tool.Define an input schema for
GetMemoriesNearPositionTool:class GetMemoriesNearPositionToolInput(BaseModel): x: float y: float z: float class GetMemoriesNearPositionTool(BaseTool): # ... args_schema: Type[GetMemoriesNearPositionToolInput] = GetMemoriesNearPositionToolInput
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/rai/rai/apps/__init__.py(1 hunks)src/rai/rai/apps/spatial_temporal_navigation/__init__.py(1 hunks)src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py(1 hunks)src/rai/rai/apps/spatial_temporal_navigation/tools.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-and-test-ros2 (humble)
- GitHub Check: build-and-test-ros2 (jazzy)
🔇 Additional comments (4)
src/rai/rai/apps/spatial_temporal_navigation/tools.py (1)
75-89: Handle Empty Results inget_memories_near_textThe function assumes that results are found in the vector store. If no results are found, this could lead to errors when querying MongoDB with an empty list of UUIDs.
[error_handling]
Add a check for empty results:
uuids = [result[0].metadata["uuid"] for result in results] +if not uuids: + return []src/rai/rai/apps/spatial_temporal_navigation/__init__.py (3)
1-13: LGTM! License header is properly formatted.The Apache 2.0 license header is correctly included with up-to-date copyright information.
27-36: LGTM! Well-structured public interface.The
__all__list properly defines the module's public interface, exposing all the necessary components for the spatiotemporal navigation system:
- Spatial components (
Pose,Orientation,PositionStamped)- Observation components (
ImageStamped,Scene,Description,Observation)- Main functionality (
run_spatial_temporal_data_collection)
16-25: Verify the imported module implementation.Let's ensure the imported module exists with all the referenced classes and function.
✅ Verification successful
All imported entities are properly implemented
The module contains all the required Pydantic model classes and the data collection function with correct signatures.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the spatial_temporal_navigation module and its components # Check if the module file exists fd -t f "spatial_temporal_navigation.py$" src/rai/rai/apps/spatial_temporal_navigation/ # Verify all imported classes and function are defined ast-grep --pattern 'class $CLASS:' src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py ast-grep --pattern 'def run_spatial_temporal_data_collection' src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.pyLength of output: 418
Script:
#!/bin/bash # Check the actual content of the file cat src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py # Try alternative patterns for class definitions ast-grep --pattern '@dataclass class $CLASS' # Look for any function definition containing our target name rg "def.*run_spatial_temporal_data_collection" -A 1Length of output: 7978
src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py
Outdated
Show resolved
Hide resolved
src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py
Outdated
Show resolved
Hide resolved
src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py
Show resolved
Hide resolved
src/rai/rai/apps/spatial_temporal_navigation/spatial_temporal_navigation.py
Show resolved
Hide resolved
| class GetMemoriesNearPositionTool(BaseTool): | ||
| name: str = "get_memories_near_position" | ||
| description: str = ( | ||
| "Get memories near a specific position using regular coordinate comparison" | ||
| ) | ||
| args_schema: Type[Pose] = Pose | ||
|
|
||
| observations_collection: Collection | ||
|
|
||
| def _run(self, pose: Pose) -> List[Observation]: | ||
| return get_memories_near_position(self.observations_collection, pose) | ||
|
|
There was a problem hiding this comment.
Correct Return Type Annotation in GetMemoriesNearPositionTool
The _run method should return a string as per the BaseTool class specification in LangChain tools, but it's currently returning a list of Observation objects.
Modify the _run method to return serialized data:
def _run(self, pose: Pose) -> str:
observations = get_memories_near_position(self.observations_collection, pose)
return json.dumps([obs.dict() for obs in observations])Ensure that you import json and adjust the return types accordingly.
Committable suggestion skipped: line range outside the PR's diff.
maciejmajek
force-pushed
the
feat/spatial-temporal-navigation
branch
from
7917be6 to
77fcc61
Compare
Purpose
One approach to maintaining a coherent long-term history is to use spatiotemporal vector stores or databases. These databases contain information such as images, descriptions, observed objects, timestamps, and 3D world positions.
Proposed Changes
This PR introduces a spatiotemporal navigation system based on:
Simplified idea:
Feature description:
API for data collection and processing, allowing for building tempo-spatial database during robot's runtime.
MongoDB stores observations, consisting of image, timestamps, position, description and so on.
Weaviate stores embeddings of textual information of observations.
Tools for querying the databases (names will most likely by changed later):
Queries MongoDB for any memories near the position
Queries MongoDB for any memories close to the timestamp
Queries Weaviate (using similarity search) for memories related to text information.
Issues
Testing
Summary by CodeRabbit
New Features
Improvements