-
Notifications
You must be signed in to change notification settings - Fork 112
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
Port pointcloud creation to numpy. #175
Conversation
Repeating my comments from ros2/geometry2#507:
|
Why is the DCO failing? Github shows my commits as correctly signed. |
It looks like you signed the commits with your GPG key, but you need to add a Signed-off-by line in your commit message. Check here: https://docs.ros.org/en/rolling/Contributing/Developer-Guide.html#change-control-process |
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
…iple field datatypes Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Duplicate of #155, but this PR has a bit more features and is activly worked on. |
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
… check fail because list!=tuple Signed-off-by: Florian Vahl <[email protected]>
While the CI still fails with some error related to python3.9 vs python3.10 in Jammy all tests are passing locally on my Ubuntu 20.04 ROS rolling machine. |
Some performance testing: As data, I used an unstructured NumPy array containing 480000 points. Serialization using the old code: |
Yeah, that's fine. This is currently broken across the board (we are waiting for some fixes in Jammy itself). Once we are closer to having this approved, we'll run CI on https://ci.ros2.org to see the status. |
I plan to write some more tests regarding organized point clouds as well as ones with mixed dtypes. |
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Signed-off-by: Florian Vahl <[email protected]>
Any other comments regarding this PR? I think all mayor issues are addressed or moved upstream without affecting this PR. |
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.
LGTM pending passing CI.
CI has picked up a small pile of flake8 linting errors.
|
Signed-off-by: Florian Vahl <[email protected]>
@gbiggs I hopefully fixed all flake8 errors |
Signed-off-by: Florian Vahl <[email protected]>
I'll definitely need to investigate why my flake8 config is so different to the one used by the CI. But I am now more or less confident that the imports are ordered correctly. |
Finally ^^ |
Thanks for the contribution! |
Implements the point cloud transformation in NumPy. This speeds up the transformation operation by 20000% for large point clouds (excluding the serialization and deserialization). The code needs some further testing but should work as it is. I also try to improve the performance of the serialization and deserialization as seen in ros2/common_interfaces#175. This adds NumPy as a dependency and drops the dependency on PyKDL which should solve #471. The API is also improved by adding proper interface descriptions. Signed-off-by: Florian Vahl <[email protected]> Co-authored-by: Abrar Rahman Protyasha <[email protected]>
This pull request ports the
PointCloud
creation to NumPy. It should massively increase the performance for large point clouds as no iteration over the points in python is needed and the bytes are taken directly from NumPy.Still needs some work regarding other field types. Feel free to comment on my implementation. Suggestions are welcome.
Cheers