[Bug][Feature] Added more missing FP16 specializations #4140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
To trigger regression tests:
|
ndickson-nvidia
force-pushed
the
more_half
branch
from
3d913a9
to
ad8972c
Compare
isratnisa
reviewed
Jun 23, 2022
src/array/cuda/array_index_select.cu
Outdated
| #ifdef USE_FP16 | ||
| // The initialization constructor for __half is apparently a device- | ||
| // only function in some setups, but the current function isn't run | ||
| // on the device. |
There was a problem hiding this comment.
I am not clear which 'current' function we are talking here.
There was a problem hiding this comment.
"the current function" here refers to the function containing this comment, the function that is currently being run (on the host) as this comment is passed. If I referred to it by name instead, i.e. IndexSelect, it might sound like the comment is referring to the other function named IndexSelect, above. Maybe it would be clear if I included both, though. 🤔 I'll try something when I add the "TODO"s.
There was a problem hiding this comment.
I updated the comment. Hopefully it's a bit clearer now. Thanks!
ndickson-nvidia
force-pushed
the
more_half
branch
from
c36e7fb
to
c425866
Compare
nv-dlasalle
approved these changes
Jun 24, 2022
…IndexSelect`, `Full`, `Scatter_`, `CSRGetData`, `CSRMM`, `CSRSum`, `IndexSelectCPUFromGPU` * Fixed casting issue in `_LinearSearchKernel` that was preventing it from supporting `__half` * Added `#if`'d out specializations of `CSRGEMM`, `CSRGEAM`, and `Xgeam`, which would require functions that aren't currently provided by cublas
…ations of Xgeam, CSRGEMM, and CSRGEAM
* Added clearer comment explaining why the cast to long long is necessary
…f can't be constructed on the host side
* Also changed the existing Xgeam function for unsupported data types from LOG(INFO) to LOG(FATAL)
ndickson-nvidia
force-pushed
the
more_half
branch
from
7418f5c
to
83d41e9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
__halfofDLDataTypeTraits,IndexSelect,Full,Scatter_,CSRGetData,CSRMM,CSRSum_LinearSearchKernelthat was preventing it from supporting__halfXgeam,CSRGEMM, andCSRGEAM, which would require functions that aren't provided by cublasChecklist
or have been fixed to be compatible with this change