apache / arrow-nanoarrow

Helpers for Arrow C Data & Arrow C Stream interfaces
https://arrow.apache.org/nanoarrow
Apache License 2.0
169 stars 35 forks source link

feat: Add Meson support in nanoarrow_device #484

Closed WillAyd closed 3 months ago

WillAyd commented 3 months ago

Sweet thanks for giving this a shot

Metal C++ is its own thing and is a sort of header-only wrapper around the Objective C (not included directly in the framework):

Possibly, though I would still hope Meson could handle that. I found another project in the WrapDB that uses Metal:

https://github.com/mesonbuild/wrapdb/blob/2d9ed3ad5bed879d8aa010de21ae165f6cf5ce4d/subprojects/packagefiles/imgui/meson.build#L49

Maybe we just need to add required: get_option('metal') to our dependency call?

...and this is maybe because cublas is not the right thing to link to (the thing we're using is called the "driver" API, which is not the "runtime" API). Also possibly because there needs to be a -DNANOARROW_WITH_CUDA compile define to get the ArrowDeviceCuda symbol into nanoarrow_device.so (still workshopping the final version of that).

Ah OK...apologies as I know embarassingly little about CUDA. But in the configuration now I have modules: [cublas] as I just copy-pasted that from the Meson documentation without much thought. Maybe that needs to be modules: [cuda-driver]?

WillAyd commented 3 months ago

Ah ignore my comment above about Metal - I confused required: get_option('metal') for something else. The only other difference I see in that case is the AppKit framework inclusion, but that seems unlikely to be the root cause.

I'll ask on the Meson IRC channel tomorrow how to resolve that header; they are usually really responsive to questions like this

WillAyd commented 3 months ago

Actually was able to borrow a MacBook to test out the metal stuff. Didn't realize metal-cpp was not installed with the system framework :-)

So the metal stuff should be compiling now. There were also some issues with the source files that needed to be corrected. Note that the metal test fails - I haven't looked into why but things at least compile now:

1/1 nanoarrow-device-metal        FAIL            0.03s   killed by signal 11 SIGSEGV
>>> MSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 ASAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1 MALLOC_PERTURB_=159 UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1:print_summary=1:print_stacktrace=1 /Users/willayd/clones/arrow-nanoarrow/builddir/src/nanoarrow/nanoarrow-device-metal-test
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀  ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
Running main() from /tmp/googletest-20230817-5025-15wtgyq/googletest-1.14.0/googletest/src/gtest_main.cc
[==========] Running 10 tests from 2 test suites.
[----------] Global test environment set-up.
[----------] 6 tests from NanoarrowDeviceMetal
[ RUN      ] NanoarrowDeviceMetal.DefaultDevice
../src/nanoarrow/nanoarrow_device_metal_test.cc:30: Failure
Expected equality of these values:
  ArrowDeviceMetalInitDefaultDevice(device.get(), nullptr)
    Which is: 22
  0

[  FAILED  ] NanoarrowDeviceMetal.DefaultDevice (15 ms)
[ RUN      ] NanoarrowDeviceMetal.DeviceGpuBufferInit
paleolimbot commented 3 months ago

Actually was able to borrow a MacBook to test out the metal stuff

Thanks! I can also help here since I have the setup. I wonder if you have an Intel MacBook (which wouldn't have a metal default device). (Metal's default device is also available on the MacOS CI runner if you use macOS-latest, since those are all M1s).

WillAyd commented 3 months ago

Yea its an old Intel device - so is the metal-cpp header only required for that?

paleolimbot commented 3 months ago

is the metal-cpp header only required for that?

It will build without error because the framework is installed; however, it won't run because there is no GPU on an intel mac (i.e., when you call GetDefaultDevice from Metal it will give you a nullptr). I'll give both of these a try shortly!