Enable Embree build with SYCL support and OneAPI 2024.1.0 #6808

lumurillo · 2024-05-31T21:25:25Z

Enable the SYCL support for the Embree package.

Type

Bug fix (non-breaking change which fixes an issue): Fixes #
New feature (non-breaking change which adds functionality). Resolves #
Breaking change (fix or feature that would cause existing functionality to not work as expected) Resolves #

Motivation and Context

This change will allow the implementation of SYCL GPU implementation for the RayCastingScene class.

Checklist:

I have run python util/check_style.py --apply to apply Open3D code style
to my code.
This PR changes Open3D behavior or adds new functionality.
- Both C++ (Doxygen) and Python (Sphinx / Google style) documentation is
  updated accordingly.
- I have added or updated C++ and / or Python unit tests OR included test
  results (e.g. screenshots or numbers) here.
I will follow up and update the code if CI fails.
For fork PRs, I have selected Allow edits from maintainers.

Description

Updated the OneAPI version for the Docker files.
Updated the CMake version.
Disable the build of the ISPC module.

update-docs · 2024-05-31T21:25:30Z

Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes.

ssheorey · 2024-06-03T23:36:21Z

Hi @lumurillo can you update the embree Python examples to use SYCL devices, if available?

examples/python/geometry/ray_casting_closest_geometry.py
examples/python/geometry/ray_casting_sdf.py
examples/python/geometry/ray_casting_to_image.py

Switch to standard sycl:: namespace for sycl API Rename internal open3d namespace to open3d::core::sy to avoid name collision TODO: Replace deprecated SYCL APIs

…neapi-2024.1.0

cpp/open3d/t/geometry/RaycastingScene.h

ssheorey · 2024-10-09T19:03:51Z

cpp/open3d/t/geometry/RaycastingScene.h

+    struct CPUImpl;
+#ifdef BUILD_SYCL_MODULE
+    struct SYCLImpl;
+#endif


Same issue here. To keep the ABI same for sycl and C++ builds, use a single pointer to DevImpl (device impl.) instead of keeping 2 structs. In the ctor, we can initialize this to CPUImpl * or SYCLImpl* appropriately.

I think it would be good to keep both, e.g. if the user explicitly wants to choose CPU, despite having a SYCL device. In this case, let's keep both all the time.

I would recommend changing each XImpl to XImpl* (for all), in line with the PIMPL pattern (pointer to implementation). This ensures the ABI doesn't change later, even if we add data to the XImpl structs.

ssheorey · 2024-10-09T19:07:25Z

cpp/open3d/t/geometry/RaycastingScene.cpp

+typedef Eigen::Matrix<float, 2, 1, Eigen::DontAlign> Vec2f;
+typedef Eigen::Vector3f Vec3f;
+
+void enablePersistentJITCache()


cpp/open3d/t/geometry/RaycastingScene.cpp

Use sycl_target_sources for files with sycl code in cmake. style fix

ssheorey · 2024-10-09T23:53:21Z

cpp/open3d/t/geometry/RaycastingScene.cpp

+#else
+    setenv("SYCL_CACHE_PERSISTENT","1",1);
+    setenv("SYCL_CACHE_DIR","cache",1);
+#endif


Move this to SYCLUtils.cpp and enable the user to enable and disable caching. Also, we should not overwrite user preferences (if they have already set these env vars, do not change them). The SYCL_CACHE_DIR should be somewhere in the home folder. For reference, here is the CUDA ptx cache default folders:
https://developer.nvidia.com/blog/cuda-pro-tip-understand-fat-binaries-jit-caching/

Perhaps $HOME/.sycl/ComputeCache/?

Defaults for sycl:
https://github.com/intel/llvm/blob/sycl/sycl/doc/EnvironmentVariables.md.
I think we should not change the default cache location. Enabling cache by default makes sense.

ssheorey

Hi Luis, looks good! See some review comments below.

[Performance] About queue.wait_and_throw() - I would recommend avoiding it in each function. Instead have a sycl utils level wait_and_throw(). This lets users launch a raycast task on the GPU, do something else on the CPU (maybe read more data, etc.) then do a queue.wait_and_throw() to get results.

The current alternative would be multithreading (check if GIL is released for raycast ops in Python), but this needs quite a bit more effort from the user.

ssheorey · 2024-11-11T22:46:36Z

cpp/open3d/t/geometry/RaycastingScene.cpp

@@ -12,10 +12,14 @@
 #include "open3d/t/geometry/RaycastingScene.h"

 // This header is in the embree src dir (embree/src/ext_embree/..).


move comment

ssheorey · 2024-11-12T23:01:39Z

cpp/open3d/t/geometry/RaycastingScene.cpp

+#else
+    setenv("SYCL_CACHE_PERSISTENT","1",1);
+    setenv("SYCL_CACHE_DIR","cache",1);
+#endif


Defaults for sycl:
https://github.com/intel/llvm/blob/sycl/sycl/doc/EnvironmentVariables.md.
I think we should not change the default cache location. Enabling cache by default makes sense.

ssheorey · 2024-11-13T00:48:41Z

cpp/open3d/t/geometry/RaycastingScene.h

+    struct CPUImpl;
+#ifdef BUILD_SYCL_MODULE
+    struct SYCLImpl;
+#endif


I think it would be good to keep both, e.g. if the user explicitly wants to choose CPU, despite having a SYCL device. In this case, let's keep both all the time.

I would recommend changing each XImpl to XImpl* (for all), in line with the PIMPL pattern (pointer to implementation). This ensures the ABI doesn't change later, even if we add data to the XImpl structs.

ssheorey · 2024-11-13T00:52:10Z

cpp/open3d/t/geometry/RaycastingScene.cpp

[Optional] Does it make sense to move the SYCL code to a new file RaycastingSceneSYCL.cpp?

ssheorey · 2024-11-13T00:55:23Z

cpp/open3d/t/geometry/RaycastingScene.cpp

+        queue_.wait_and_throw();
+
+        // Free the allocated memory
+        sycl::free(previous_geom_prim_ID_tfar, queue_);


Is this a memory leak, if there's an exception in the queue above? To prevent that, set previous_geom_prim_ID_tfar=nullptr; after free and do another free in the SYCLImpl dtor if the ptr is not null.

ssheorey · 2024-11-13T01:02:16Z

cpp/open3d/t/geometry/RaycastingScene.cpp

@@ -689,16 +1169,50 @@ struct RaycastingScene::Impl {
                    LoopFn);
        }
    }
+


Can we use STL here for ArraySum and ArrayPartialSum, and memcpy for CopyArray?

ssheorey · 2024-11-18T07:04:27Z

python/test/t/geometry/test_raycasting_scene.py

@@ -122,14 +122,10 @@ def test_test_lots_of_occlusions(device):
 def test_add_triangle_mesh(device):
    cube = o3d.t.geometry.TriangleMesh.from_legacy(


Can remove from_legacy for mesh creation functions now.

ssheorey · 2024-11-18T15:34:23Z

Some updates needed to pass the CUDA CI tests. I made a branch copy and ran them here:
https://github.com/isl-org/Open3D/actions/runs/11887514382

@lumurillo I'll take care of this.

pybind some SYCL util functions.

ssheorey · 2024-11-19T06:48:07Z

Looks good. We can visit the wait_and_throw() optimization later. Thanks @lumurillo !

benjaminum

lgtm

ssheorey marked this pull request as draft June 14, 2024 19:21

ssheorey self-requested a review June 14, 2024 19:21

lumurillo and others added 13 commits October 4, 2024 08:59

Build Embree with SYCL support and OneAPI 2024.1.0

cd841a8

Add correct compiler for embree SYCL

97402f9

Add embree dependency change

5d90e33

Fix embree linking error

c799893

Switch to standard sycl:: namespace for sycl API Rename internal open3d namespace to open3d::core::sy to avoid name collision TODO: Replace deprecated SYCL APIs

Fix SYCL demo typo

f3bde48

Fix ubuntu CI

d37e8c1

Fix code style

667dc18

Fix python virtualenv in Dockerfile

94c0f0f

Update oneAPI version to SYCL shared process

f1515e2

Add initial SYCL support to RayCastingScene class

bd02607

Add SYCL support to CountIntersections function

2557482

Provide SYCL support to list intersections functions WIP

2689d45

Remove open3d python virtualenv from Dockerfile

048008c

lumurillo force-pushed the lumurillo/use-oneapi-2024.1.0 branch from 94a7db2 to 048008c Compare October 4, 2024 17:35

Merge branch 'main' of github.com:isl-org/Open3D into lumurillo/use-o…

84f0ef9

…neapi-2024.1.0

ssheorey reviewed Oct 9, 2024

View reviewed changes

Fix no tbb target with BUILD_SYCL_MODULE

e25a9b7

Use sycl_target_sources for files with sycl code in cmake. style fix

ssheorey reviewed Oct 9, 2024

View reviewed changes

ssheorey force-pushed the lumurillo/use-oneapi-2024.1.0 branch 2 times, most recently from cb71f22 to aa8f03e Compare October 10, 2024 17:38

Add array utils functions

d3e68ea

lumurillo force-pushed the lumurillo/use-oneapi-2024.1.0 branch from aa8f03e to d3e68ea Compare October 12, 2024 17:50

lumurillo added 4 commits October 13, 2024 11:24

Set the SYCL ArrayPartialSum as a sequential implementation

09860d0

Make the ListIntersections function work on SYCL

487596f

Fix ListInteractions SYCL kernel

975e7f0

Add list intersections test

c0c2aea

lumurillo added 3 commits November 2, 2024 11:34

Prepare ComputeClosestPoints for SYCL implementation

278295c

Add SYCL enable parameter to list_devices

aa437b6

Fix SYCL version of CountIntersections function

1eec474

ssheorey added this to the v0.19 milestone Nov 6, 2024

lumurillo added 3 commits November 8, 2024 16:20

Fix Dockerfile

2c98133

Fix code style

7b34720

Simplify RayCastingScene constructor

6284e3d

lumurillo force-pushed the lumurillo/use-oneapi-2024.1.0 branch from f4c3a26 to 6284e3d Compare November 9, 2024 14:07

lumurillo marked this pull request as ready for review November 9, 2024 17:00

ssheorey reviewed Nov 13, 2024

View reviewed changes

ssheorey requested a review from benjaminum November 13, 2024 15:13

lumurillo and others added 3 commits November 14, 2024 14:57

Move enable JIT cache function to SYCL utils

460e3f7

Do not change the SYCL cache dir

8da0afc

test RaycastingScene.add_triangles with TriangleMesh

8dbb6fa

ssheorey reviewed Nov 18, 2024

View reviewed changes

create objects for testing with tensor TriangleMesh

23b8c7d

lumurillo and others added 5 commits November 18, 2024 16:21

Fix possible memory leak in SYCL implementation of RayCastingScene

4b81b70

Fix code style

5b6a6de

Use STL functions in CPU array operations for RayCastingScene

b2689cb

Move comment

239ccf8

Do not include CUDA devices in raycast tests.

9ef538a

pybind some SYCL util functions.

ssheorey approved these changes Nov 19, 2024

View reviewed changes

benjaminum approved these changes Nov 19, 2024

View reviewed changes

Convert deprecated SYCL 1.2 code to SYCL 2020

e3baed6

ssheorey merged commit 5f4985b into isl-org:main Nov 20, 2024
29 of 33 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable Embree build with SYCL support and OneAPI 2024.1.0 #6808

Enable Embree build with SYCL support and OneAPI 2024.1.0 #6808

lumurillo commented May 31, 2024 •

edited

Loading

update-docs bot commented May 31, 2024

ssheorey commented Jun 3, 2024

ssheorey Oct 9, 2024

ssheorey Nov 13, 2024

ssheorey Oct 9, 2024

lumurillo Nov 14, 2024

ssheorey Oct 9, 2024

ssheorey Nov 12, 2024

lumurillo Nov 14, 2024

ssheorey left a comment

ssheorey Nov 11, 2024

ssheorey Nov 12, 2024

ssheorey Nov 13, 2024

ssheorey Nov 13, 2024

ssheorey Nov 13, 2024

lumurillo Nov 19, 2024

ssheorey Nov 13, 2024

lumurillo Nov 19, 2024

ssheorey Nov 18, 2024

ssheorey commented Nov 18, 2024

ssheorey commented Nov 19, 2024

benjaminum left a comment

		@@ -12,10 +12,14 @@
		#include "open3d/t/geometry/RaycastingScene.h"

		// This header is in the embree src dir (embree/src/ext_embree/..).

		@@ -122,14 +122,10 @@ def test_test_lots_of_occlusions(device):
		def test_add_triangle_mesh(device):
		cube = o3d.t.geometry.TriangleMesh.from_legacy(

Enable Embree build with SYCL support and OneAPI 2024.1.0 #6808

Enable Embree build with SYCL support and OneAPI 2024.1.0 #6808

Conversation

lumurillo commented May 31, 2024 • edited Loading

Type

Motivation and Context

Checklist:

Description

update-docs bot commented May 31, 2024

ssheorey commented Jun 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssheorey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssheorey commented Nov 18, 2024

ssheorey commented Nov 19, 2024

benjaminum left a comment

Choose a reason for hiding this comment

lumurillo commented May 31, 2024 •

edited

Loading