-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
Add reduction param to pypose.functional.reprojerr #260
Add reduction param to pypose.functional.reprojerr #260
Conversation
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.
Although not compulsory, I prefer using single quotation marks '
instead of double "
for strings, as we only need to press one button for single quotation mark and need to press 'shift' button for double quotation mark.
pypose/function/geometry.py
Outdated
@@ -126,8 +126,14 @@ def reprojerr(points, pixels, intrinsics, extrinsics=None): | |||
The shape has to be (..., 3, 3). | |||
extrinsics (``LieTensor``, optional): The camera extrinsics. | |||
The shape has to be (..., 7). Default: ``None``. | |||
reduction (``str``, optional): The reduction to apply on the output: ``"none"`` | ``"sum"`` | ``"norm"`` |
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.
Line exceeding 90 chars, see point 2 of this guideline.
pypose/function/geometry.py
Outdated
@@ -126,8 +126,14 @@ def reprojerr(points, pixels, intrinsics, extrinsics=None): | |||
The shape has to be (..., 3, 3). | |||
extrinsics (``LieTensor``, optional): The camera extrinsics. | |||
The shape has to be (..., 7). Default: ``None``. | |||
reduction (``str``, optional): The reduction to apply on the output: ``"none"`` | ``"sum"`` | ``"norm"`` | |||
``"none"``: No reduction is applied | |||
``"sum"``: The reprojection error on each component (u, v) is summed for each pixel (L1 Norm) |
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.
line too long
pypose/function/geometry.py
Outdated
tensor([0., 0., 0., 0., 0., 0.]) | ||
''' | ||
torch.broadcast_shapes(points.shape[:-2], pixels.shape[:-2], intrinsics.shape[:-2]) | ||
assert points.size(-1) == 3 and pixels.size(-1) == 2 and \ | ||
intrinsics.size(-1) == intrinsics.size(-2) == 3, "Shape not compatible." | ||
assert reduction in {"norm", "sum", "none"}, "Reduction method can only be 'norm'|'sum'|'none'." |
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.
line too long
pypose/function/geometry.py
Outdated
| ``'sum'`` | ``'norm'`` | ||
``'none'``: No reduction is applied | ||
``'sum'``: The reprojection error on each component (u, v) is summed for | ||
each pixel (L1 Norm) | ||
``'norm'``: The reprojection error's L2 norm for each pixel |
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.
It seems that you need commas for every line, otherwise, those lines are connected automatically by docstring. See this auto-generated doc for your PR:
https://pypose--260.org.readthedocs.build/en/260/generated/pypose.reprojerr.html
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.
You may generate docs locally following this readme.
Add parameter
reduction
topypose.function.reprojerr
.