Skip to content

ARROW-3209: [C++] Rename libarrow_gpu to libarrow_cuda#3088

Closed
pitrou wants to merge 7 commits intoapache:masterfrom
pitrou:ARROW-3209-rename-arrow-gpu-to-cuda
Closed

ARROW-3209: [C++] Rename libarrow_gpu to libarrow_cuda#3088
pitrou wants to merge 7 commits intoapache:masterfrom
pitrou:ARROW-3209-rename-arrow-gpu-to-cuda

Conversation

@pitrou
Copy link
Member

@pitrou pitrou commented Dec 4, 2018

Also rename the arrow::gpu namespace to arrow::cuda.

@pitrou
Copy link
Member Author

pitrou commented Dec 4, 2018

I think the C glib side needs to be fixed, but I'm not sure how. @kou would you like to push something to this PR?

Also rename the arrow::gpu namespace to arrow::cuda.
@pitrou pitrou force-pushed the ARROW-3209-rename-arrow-gpu-to-cuda branch from 7e6e1aa to 9687346 Compare December 4, 2018 17:22
@wesm
Copy link
Member

wesm commented Dec 4, 2018

Ah, I forgot about the disruption that this would cause to the glib side, where there are also Linux packages. I think it's a good idea to leave open the possibility of GPU computing outside of CUDA

@kou
Copy link
Member

kou commented Dec 5, 2018

I'll add some commits for C GLib, Ruby and Linux packages to this PR.

@kou
Copy link
Member

kou commented Dec 5, 2018

It seems that Cuda in arrow::cuda::Cuda* is redundant.
Should we keep Cuda or remove Cuda?

@codecov-io
Copy link

codecov-io commented Dec 5, 2018

Codecov Report

Merging #3088 into master will increase coverage by 1.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3088      +/-   ##
==========================================
+ Coverage   87.12%   88.15%   +1.03%     
==========================================
  Files         492      434      -58     
  Lines       69104    65339    -3765     
==========================================
- Hits        60204    57601    -2603     
+ Misses       8801     7738    -1063     
+ Partials       99        0      -99
Impacted Files Coverage Δ
cpp/src/plasma/client.cc 84.68% <ø> (ø) ⬆️
cpp/src/plasma/protocol.cc 95.62% <ø> (ø) ⬆️
cpp/src/plasma/plasma.h 100% <ø> (ø) ⬆️
cpp/src/plasma/store.h 100% <ø> (ø) ⬆️
cpp/src/plasma/store.cc 92.42% <ø> (ø) ⬆️
cpp/src/plasma/common.h 100% <ø> (ø) ⬆️
cpp/src/plasma/test/client_tests.cc 100% <ø> (ø) ⬆️
go/arrow/array/table.go
go/arrow/math/uint64_amd64.go
go/arrow/internal/testing/tools/bool.go
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4ac4eb5...219ffa2. Read the comment docs.

@pitrou
Copy link
Member Author

pitrou commented Dec 5, 2018

It seems that Cuda in arrow::cuda::Cuda* is redundant. Should we keep Cuda or remove Cuda?

I'd keep it.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.
+1 from me.

We can merge this when Linux packages are built on Travis CI:
https://github.com/kou/crossbow/branches/all?utf8=%E2%9C%93&query=build-53

@wesm
Copy link
Member

wesm commented Dec 5, 2018

Some of the Linux packages failed

@kou
Copy link
Member

kou commented Dec 6, 2018

Linux package builds are fixed: https://github.com/kou/crossbow/branches/all?utf8=%E2%9C%93&query=build-54

I'll merge this when CI is passed.

@wesm
Copy link
Member

wesm commented Dec 6, 2018

Great, thanks @kou!

@kou
Copy link
Member

kou commented Dec 6, 2018

CI is green. I'll merge this.

@kou kou closed this in 898e06c Dec 6, 2018
@pitrou pitrou deleted the ARROW-3209-rename-arrow-gpu-to-cuda branch December 6, 2018 09:11
shiro615 pushed a commit that referenced this pull request Dec 12, 2018
This is a follow-up change for #3088.

Author: Kouhei Sutou <kou@clear-code.com>

Closes #3162 from kou/glib-replace-gpu-with-cuda and squashes the following commits:

8891e51 <Kouhei Sutou>  Replace GPU with CUDA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants