List intersections #6442
List intersections #6442
Conversation
|
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
There was a problem hiding this comment.
Thanks @dmitrishastin ! I think this is a good contribution. Please have a look at the comments. Other things that are missing is an example inside the python docstring and a unit test for the new function. Let me know if you need help with this.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @dmitrishastin)
cpp/open3d/t/geometry/RaycastingScene.cpp line 553 at r1 (raw file):
const size_t num_rays, const size_t num_intersections, Eigen::VectorXi cumsum,
Pass cumsum as const reference to avoid copy
cpp/open3d/t/geometry/RaycastingScene.cpp line 855 at r1 (raw file):
Eigen::Map<Eigen::VectorXi> intersections_vector( intersections.GetDataPtr<int>(), num_rays); Eigen::Map<Eigen::VectorXi> num_intersections(
Isn't the temporary Tensor destroyed here and the pointer invalid?
cpp/pybind/t/geometry/raycasting_scene.cpp line 203 at r1 (raw file):
ray_ids A tensor with ray IDs. The shape is {..}.
I think it would be easier to use the result if the cumsum. It would be similar to here http://www.open3d.org/docs/release/python_api/open3d.ml.tf.layers.RadiusSearch.html where each query can have a different number of neighbors. @dmitrishastin what is your opinion here?
There was a problem hiding this comment.
Thank you for reviewing promptly @benjaminum ! I addressed the comments to the best of my ability. I have also included the example inside the python docstring as suggested, and checked it by compiling documentation. I am, however, not clear regarding the unit test as I could not find any unit tests for RaycastingScene - your help would be appreciated. Finally, I have expanded my original submitted function to also output primitive_uvs as this made sense.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @benjaminum)
cpp/open3d/t/geometry/RaycastingScene.cpp line 553 at r1 (raw file):
Previously, benjaminum (Benjamin Ummenhofer) wrote…
Pass cumsum as const reference to avoid copy
Done.
cpp/open3d/t/geometry/RaycastingScene.cpp line 855 at r1 (raw file):
Previously, benjaminum (Benjamin Ummenhofer) wrote…
Isn't the temporary Tensor destroyed here and the pointer invalid?
Thanks for spotting! I have changed so it hopefully makes more sense.
cpp/pybind/t/geometry/raycasting_scene.cpp line 203 at r1 (raw file):
Previously, benjaminum (Benjamin Ummenhofer) wrote…
I think it would be easier to use the result if the cumsum. It would be similar to here http://www.open3d.org/docs/release/python_api/open3d.ml.tf.layers.RadiusSearch.html where each query can have a different number of neighbors. @dmitrishastin what is your opinion here?
Apologies - I am running into trouble compiling the ML module on Windows. Would you be able to provide an example of RadiusSearch output for me to be more clear on your proposed alternative? Thank you!
There was a problem hiding this comment.
The unit tests for RaycastingScene are using the python bindings and are here https://github.com/isl-org/Open3D/blob/master/python/test/t/geometry/test_raycasting_scene.py . Adding primitive_uvs makes sense.
Thanks again and see the new comments!
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @dmitrishastin)
cpp/open3d/t/geometry/RaycastingScene.cpp line 553 at r1 (raw file):
Previously, dmitrishastin (Dmitri Šastin) wrote…
Done.
The & is missing to avoid the copy -> const Eigen::VectorXi& cumsum
cpp/pybind/t/geometry/raycasting_scene.cpp line 203 at r1 (raw file):
Previously, dmitrishastin (Dmitri Šastin) wrote…
Apologies - I am running into trouble compiling the ML module on Windows. Would you be able to provide an example of
RadiusSearchoutput for me to be more clear on your proposed alternative? Thank you!
The idea is to use the cumulative sum for pointing to the start and end of the intersections for each ray similar to this example https://www.tensorflow.org/guide/ragged\_tensor#tfraggedtensorfrom\_row\_splits .
Let's say we have 5 rays with 2, 1, 3, 0, 2 intersections then the output could look like this
ray_splits : [0, 2, 3, 6, 6, 8] # note that the length of this is num_rays+1
t_hit: [t1, t2, t3, t4, t5, t6, t7, t8]
# ray_splits can be used to iterate over all intersections for each ray
for ray_id, (start, end) in enumerate(zip(ray_splits[:-1], ray_splits[1:])):
for i,t in enumerate(t_hit[start:end]):
print(f'ray {ray_id}, intersection {i} at {t}')
We can also think about having both ray_splits and ray_ids to allow iterating over rays and intersections easily.
There was a problem hiding this comment.
Thanks! Didn't realise to look in python folder.
I have added the unit tests; however, while running them, I discovered that the function isn't nearly as fast as I'd hoped - it gets pretty slow in my setting when >250K rays are used. In fact, this increase in running time appears to be exponential:
import numpy as np
import open3d as o3d
import time
cube = o3d.t.geometry.TriangleMesh.from_legacy(
o3d.geometry.TriangleMesh.create_box())
scene = o3d.t.geometry.RaycastingScene()
scene.add_triangles(cube)
rs = np.random.RandomState(123)
def time_fn(nrays):
rays = o3d.core.Tensor.from_numpy(rs.rand(nrays, 6).astype(np.float32))
start = time.time()
_ = scene.list_intersections(rays)
finish = time.time()
return finish - start
rpt_times = 5
nrays = np.array([100, 500, 1000, 5000, 10000, 50000, 100000, 150000, 200000, 250000])
runtime = np.empty((nrays.size, rpt_times))
for r in range(rpt_times):
for i, n in enumerate(nrays):
runtime[i, r] = time_fn(n)
print(np.round(runtime, 1))
[[ 0. 0. 0. 0. 0. ]
[ 0. 0. 0. 0. 0. ]
[ 0. 0. 0. 0. 0. ]
[ 0. 0. 0. 0. 0. ]
[ 0. 0. 0. 0. 0. ]
[ 0.1 0.2 0.2 0.2 0.2]
[ 0.7 1. 1. 1. 1. ]
[ 2.9 2.8 2.8 2.7 2.5]
[ 8. 7.1 8.2 8.3 5.7]
[16.1 14.1 13.3 12.2 10.8]]
rel_runtime = runtime / nrays[:, None]
print(np.round(rel_runtime, 5))
[[0.e+00 0.e+00 0.e+00 0.e+00 0.e+00]
[0.e+00 0.e+00 0.e+00 0.e+00 0.e+00]
[0.e+00 0.e+00 0.e+00 0.e+00 0.e+00]
[0.e+00 0.e+00 0.e+00 0.e+00 0.e+00]
[0.e+00 0.e+00 0.e+00 0.e+00 0.e+00]
[0.e+00 0.e+00 0.e+00 0.e+00 0.e+00]
[1.e-05 1.e-05 1.e-05 1.e-05 1.e-05]
[2.e-05 2.e-05 2.e-05 2.e-05 2.e-05]
[4.e-05 4.e-05 4.e-05 4.e-05 3.e-05]
[6.e-05 6.e-05 5.e-05 5.e-05 4.e-05]]
For now, I have simply set the number of rays in unit tests to be one order less - but will be grateful on any thoughts.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @benjaminum)
cpp/open3d/t/geometry/RaycastingScene.cpp line 553 at r1 (raw file):
Previously, benjaminum (Benjamin Ummenhofer) wrote…
The
&is missing to avoid the copy ->const Eigen::VectorXi& cumsum
Thanks - still getting to grips with c++!
cpp/pybind/t/geometry/raycasting_scene.cpp line 203 at r1 (raw file):
Previously, benjaminum (Benjamin Ummenhofer) wrote…
The idea is to use the cumulative sum for pointing to the start and end of the intersections for each ray similar to this example https://www.tensorflow.org/guide/ragged\_tensor#tfraggedtensorfrom\_row\_splits .
Let's say we have 5 rays with 2, 1, 3, 0, 2 intersections then the output could look like this
ray_splits : [0, 2, 3, 6, 6, 8] # note that the length of this is num_rays+1 t_hit: [t1, t2, t3, t4, t5, t6, t7, t8] # ray_splits can be used to iterate over all intersections for each ray for ray_id, (start, end) in enumerate(zip(ray_splits[:-1], ray_splits[1:])): for i,t in enumerate(t_hit[start:end]): print(f'ray {ray_id}, intersection {i} at {t}')We can also think about having both
ray_splitsandray_idsto allow iterating over rays and intersections easily.
Thanks for the clarification! I have added ray_splits as suggested, and included your example in python documentation. I have left ray_ids in for now - but can take it out if you think that's best.
…/Open3D into list_intersections
…ishastin/Open3D into list_intersections" This reverts commit 58abef6, reversing changes made to 9d7dccf.
|
@dmitrishastin I made some minor changes to the example. I like the intersection computation with the primitive_uvs. It is useful code also for interpolating vertex colors and other attributes and I don't think we have such an example yet. About the shape of the rays, I tested {10,10,6} but the results don't look correct. There seems to be uninitialized memory in the output. |
There was a problem hiding this comment.
Great - thank you. Using t_hit to compute intersection coordinates is a better method and should have definitely been demonstrated! As you suggest, I am using the code for interpolating surface normals at intersection points for my application, hence the original example.
Regarding shapes, I agree - I was not careful in how I handled arbitrary dimentions.
I have corrected the code - it still supports arbitrary dimentions for input but returns linear outputs as intended.
I have also made this clearer in documentation. The unit test for the shape has been updated.
Quick sanity check:
import open3d as o3d
import numpy as np
scene = o3d.t.geometry.RaycastingScene()
d = o3d.data.MonkeyModel()
_ = scene.add_triangles(o3d.t.io.read_triangle_mesh(d.path))
rs = np.random.RandomState(123)
rays = o3d.core.Tensor.from_numpy(rs.rand(4, 5, 6).astype(np.float32))
lx = scene.list_intersections(rays)
print(rays.shape)
print(lx['ray_splits'].shape)
print(lx['t_hit'].shape)
print(lx['ray_splits'])
print(lx['t_hit'])
Output:
SizeVector[4, 5, 6]
SizeVector[21]
SizeVector[12]
[0 0 1 1 2 3 4 5 5 5 6 7 7 8 8 9 9 9 10 11 11]
[0.122634 0.438293 0.0261266 0.305563 0.273778 0.0826897 0.264928 0.0486981 0.0776304 0.348788 0.550925 0.191991]
Reshape:
rays = rays.reshape((20, 6))
lx = scene.list_intersections(rays)
print(rays.shape)
print(lx['ray_splits'].shape)
print(lx['t_hit'].shape)
print(lx['ray_splits'])
print(lx['t_hit'])
Output:
SizeVector[20, 6]
SizeVector[21]
SizeVector[12]
[0 0 1 1 2 3 4 5 5 5 6 7 7 8 8 9 9 9 10 11 11]
[0.122634 0.438293 0.0261266 0.305563 0.273778 0.0826897 0.264928 0.0486981 0.0776304 0.348788 0.550925 0.191991]
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @benjaminum)
There was a problem hiding this comment.
I think only one more change to the documentation is needed and then this PR is ready for merging :)
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @dmitrishastin)
cpp/open3d/t/geometry/RaycastingScene.h line 107 at r6 (raw file):
const int nthreads = 0); /// \brief Computes the closest points on the surfaces of the scene.
The documentation ListIntersections and ComputeClosestPoints are mixed up
cpp/open3d/t/geometry/RaycastingScene.h line 128 at r6 (raw file):
const core::Tensor &rays, const int nthreads = 0); /// \brief Lists the intersections of the rays with the scene
The documentation ListIntersections and ComputeClosestPoints are mixed up
There was a problem hiding this comment.
So this is confusing. I believe CountIntersections docs were already incorrect in the main branch.
While preparing my previous commit, I noticed only a part of this and I updated accordingly. After pushing the commit, I realised that the whole CountIntersections text was incorrect and not just that part. I then decided to simply revert documentation to its original text and not touch CountIntersections at all as this had nothing to do with my own PR. Admittedly, this required another commit that I pushed after replying to your last comment, and you may not have seen it.
Overall, I only changed RaycastingScene.h by adding ListIntersections, as is seen in the files section of the PR. I believe that the final documentation for ListIntersection is correct. Please let me know if I've missed anything. I am also happy to correct documentation for CountIntersections as part of my PR - should you deem this appropriate, of course!
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @benjaminum)
There was a problem hiding this comment.
Documentation is fixed now.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @dmitrishastin)
There was a problem hiding this comment.
Many thanks for all your help!
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @benjaminum)
Add "ListIntersections" (C++) / "list_intersections" (python) function that returns a dictionary for all ray intersections present in a raycasting scene.
Type
Motivation and Context
Flexibility in ray inclusion / exclusion.
Checklist:
python util/check_style.py --applyto apply Open3D code styleto my code.
updated accordingly.
results (e.g. screenshots or numbers) here.
Description
Accepts a rays tensor. The returned dictionary contains:
Runs
count_intersectionsfirst to pre-allocate results arrays then executes the actual function.Can be tested with the following code:
Output:
Visualisation:
This change is