Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rename logical_cast to bit_cast and allow additional conversions #7373

Merged
merged 10 commits into from
Mar 3, 2021
Merged

Rename logical_cast to bit_cast and allow additional conversions #7373

merged 10 commits into from
Mar 3, 2021

Conversation

ttnghia
Copy link
Contributor

@ttnghia ttnghia commented Feb 11, 2021 •
edited
Loading

This MR resolves #6834. It renames logical_cast to bit_cast which performs casting following the rules of std::bit_cast: the two types (FromType and ToType) must have the same size and are trivially copyable.

@ttnghia ttnghia requested review from a team as code owners February 11, 2021 21:48
@github-actions github-actions bot added CMake CMake build issue conda Java Affects Java cuDF API. Python Affects Python cuDF API. libcudf Affects libcudf (C++/CUDA) code. labels Feb 11, 2021
@ttnghia ttnghia changed the base branch from branch-0.18 to branch-0.19 February 11, 2021 21:48
@ttnghia ttnghia removed CMake CMake build issue conda Java Affects Java cuDF API. Python Affects Python cuDF API. labels Feb 11, 2021
@ttnghia
Copy link
Contributor Author

ttnghia commented Feb 24, 2021

Pinging again on the rename from logical_cast -> bit_cast. Do we care about that? @jrhemstad

I talk with Jake about that. Now we no longer have logical_cast but bit_cast 😃

@jrhemstad jrhemstad changed the title Allow logical_cast to cast between different types having the same size Rename logical_cast to bit_cast and allow additional conversions Feb 25, 2021
@nvdbaranec nvdbaranec self-requested a review February 25, 2021 16:28
@github-actions github-actions bot added the CMake CMake build issue label Feb 25, 2021
@ttnghia ttnghia removed the CMake CMake build issue label Feb 25, 2021
@github-actions github-actions bot added the CMake CMake build issue label Mar 2, 2021
@ttnghia ttnghia removed the CMake CMake build issue label Mar 2, 2021
@ttnghia ttnghia removed the request for review from revans2 March 2, 2021 18:28
@harrism
Copy link
Member

harrism commented Mar 2, 2021

@gpucibot merge

@revans2 revans2 mentioned this pull request Mar 3, 2021
Merged
@ttnghia ttnghia requested a review from a team March 3, 2021 13:31
@rapids-bot rapids-bot bot merged commit 2f1feaa into rapidsai:branch-0.19 Mar 3, 2021
@kkraus14
Copy link
Collaborator

kkraus14 commented Mar 3, 2021

@gpucibot merge

rapids-bot bot pushed a commit that referenced this pull request Mar 3, 2021
@revans2
JNI bit cast (#7493)
c69b6f8 
In #7373 logical_cast is renamed to bit_cast. That breaks JNI, and this fixes it.

Authors:
  - Robert (Bobby) Evans (@revans2)
  - Nghia Truong (@ttnghia)

Approvers:
  - Raza Jafri (@razajafri)
  - Jason Lowe (@jlowe)
  - Alessandro Bellina (@abellina)

URL: #7493
@ttnghia ttnghia deleted the branch-0.19-bug-6834 branch March 11, 2021 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kkraus14 kkraus14 kkraus14 approved these changes

@davidwendt davidwendt davidwendt approved these changes

@nvdbaranec nvdbaranec nvdbaranec approved these changes

@jrhemstad jrhemstad Awaiting requested review from jrhemstad

Assignees

@ttnghia ttnghia

Labels
3 - Ready for Review Ready for review by team 5 - Ready to Merge Testing and reviews complete, ready to merge breaking Breaking change improvement Improvement / enhancement to an existing function libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FEA] logical_cast from FLOAT32 to INT32 and FLOAT64 to INT64 or bit_cast for the same
7 participants
@ttnghia @mythrocks @nvdbaranec @harrism @kkraus14 @jrhemstad @davidwendt