align API with psjac#31
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the internal TrackingTensor and @map_transform with PyPose's native pp.Parameter(..., sjac=True) and @psjac decorator across the codebase and documentation. Several issues were identified in the pgo.py implementation and its corresponding documentation where parameters were initialized with plain tensors instead of pp.LieTensor objects, which would prevent the intended 6D tangent-space optimization. Additionally, explicit casting to pp.SE3 is recommended within residual functions to avoid potential AttributeError when @psjac unwraps tracked parameters.
| def __init__(self, nodes): | ||
| super().__init__() | ||
| self.nodes = nn.Parameter(TrackingTensor(nodes)) | ||
| self.nodes = pp.Parameter(nodes, sjac=True) |
There was a problem hiding this comment.
Initializing pp.Parameter with a plain 7D tensor will result in 7D optimization (standard Euclidean gradient for quaternions) instead of the intended 6D tangent-space optimization. PyPose parameters only trigger manifold-aware optimization automatically when the underlying data is a pp.LieTensor.
| self.nodes = pp.Parameter(nodes, sjac=True) | |
| self.nodes = pp.Parameter(pp.SE3(nodes), sjac=True) |
| def __init__(self, nodes_rest): | ||
| super().__init__() | ||
| self.nodes_rest = nn.Parameter(TrackingTensor(nodes_rest)) | ||
| self.nodes_rest = pp.Parameter(nodes_rest, sjac=True) |
There was a problem hiding this comment.
Similar to the PoseGraph class, self.nodes_rest should be initialized with a pp.LieTensor to ensure 6D tangent-space optimization is used during the solve.
| self.nodes_rest = pp.Parameter(nodes_rest, sjac=True) | |
| self.nodes_rest = pp.Parameter(pp.SE3(nodes_rest), sjac=True) |
| super().__init__() | ||
| self.nodes = nn.Parameter(TrackingTensor(nodes)) | ||
| self.nodes.trim_SE3_grad = True | ||
| self.nodes = pp.Parameter(nodes, sjac=True) |
There was a problem hiding this comment.
The example should demonstrate initializing the parameter with a pp.LieTensor (like pp.SE3) to ensure the 6D optimization behavior described in the notes below is actually triggered.
| self.nodes = pp.Parameter(nodes, sjac=True) | |
| self.nodes = pp.Parameter(pp.SE3(nodes), sjac=True) |
|
|
||
| - `self.nodes` is typically shape `(num_nodes, 7)` in quaternion SE(3) storage. | ||
| - `trim_SE3_grad = True` converts each stored pose block into a 6D optimized tangent-space block. | ||
| - `pp.Parameter(..., sjac=True)` automatically handles the 6D tangent-space optimization. |
There was a problem hiding this comment.
This note is slightly misleading. pp.Parameter(..., sjac=True) only handles 6D tangent-space optimization automatically if the input data is already a pp.LieTensor. If a plain 7D tensor is passed, it will be treated as a 7D Euclidean parameter.
| - `pp.Parameter(..., sjac=True)` automatically handles the 6D tangent-space optimization. | |
| - `pp.Parameter(..., sjac=True)` automatically handles the 6D tangent-space optimization when initialized with a `pp.LieTensor`. |
Clarify the behavior of pp.Parameter with sjac=True regarding Jacobian recovery.
Clarified the description of the forward pass in the BAE Compute Graph.
1 participant
This pull request makes a comprehensive transition from a custom tracking and transformation system (
TrackingTensor,map_transform) to PyPose interfaces (pp.Parameterwithsjac=True, andpsjac). The changes update both documentation and code, ensuring consistency.