ingowald / cudaKDTree

206 stars 18 forks source link

ZipCompare as a host function too #9

Closed bricerebsamen closed 1 year ago

bricerebsamen commented 1 year ago

Make ZipCompare a __host__ function too. Also split updateTag into the kernel which updates all the points and the single tag update function.

I did that because I copied and modified buildTree to work on CPU.

I am pretty sure buildTree could be rewritten with some thrust magic to be able to run both on CPU and GPU by simply checking the exec policy but I haven't found a way to do that...

ingowald commented 1 year ago

currently working on merging this 'by hand' because it conflicts with other changes i had done in parallel. question: main goal of this was to allow building on the host, right? so you want to have a buildTree(...) that also runs on the host, and operates on host-read/writeable memory.... right?

ingowald commented 1 year ago

Huh. I looked at this branch of yours, but for the life of me can't figure out how you made that work on the host with the thrust::device_vectors and stuff. Are you OK if I "merge" this by adding a separate cukd/builder_host.h that uses std::vector's, std::sort, etc, but otherwise builds the same binary-compatible trees? (with the same updateTag etc)?

ingowald commented 1 year ago

should that host builder be / would it need to be parallel?

bricerebsamen commented 1 year ago

you want to have a buildTree(...) that also runs on the host, and operates on host-read/writeable memory.... right?

yes, that's correct

can't figure out how you made that work on the host with the thrust::device_vectors

I copy pasted buildTree and changed host_device to std::vector. I think that was about it: I still used thrust::sort on it, because std::sort wouldn't accept the zip comparator

And no I don't need it to build in parallel on the CPU.

ingowald commented 1 year ago

Duh; I didn't even know that thrust sort would work in the host... that makes it so much easier. I'll have a look, that shouldn't be too hard then.

bricerebsamen commented 1 year ago

@ingowald incorporated this directly in master