Skip to content

Conversation

@vishwakftw
Copy link
Contributor

Changelog:

  • Re-enable multinomial sampling when the probability tensor has dtype == torch.half.

It seems to have been missed in #28481.

Fixes #29211

Changelog:
- Re-enable multinomial sampling when the probability tensor has `dtype == torch.half`.
@vishwakftw vishwakftw requested a review from ifedan November 6, 2019 02:00
@vishwakftw vishwakftw requested a review from ezyang November 11, 2019 23:07
@ezyang
Copy link
Contributor

ezyang commented Nov 12, 2019

test?

@vishwakftw
Copy link
Contributor Author

@ezyang Tests have been added.

@ezyang
Copy link
Contributor

ezyang commented Nov 15, 2019

build failure is real

Nov 14 17:04:39 /Users/distiller/workspace/clang++  -DAT_PARALLEL_OPENMP=1 -DCAFFE2_BUILD_MAIN_LIB -DCPUINFO_SUPPORTED_PLATFORM=1 -DHAVE_MMAP=1 -DHAVE_SHM_OPEN=1 -DHAVE_SHM_UNLINK=1 -DIDEEP_USE_MKL -DNNP_CONVOLUTION_ONLY=0 -DNNP_INFERENCE_ONLY=0 -DONNX_ML=1 -DONNX_NAMESPACE=onnx_torch -DTH_BLAS_MKL -DUSE_CUDA -D_FILE_OFFSET_BITS=64 -Dtorch_EXPORTS -Iaten/src -I../aten/src -I. -I../ -I../cmake/../third_party/benchmark/include -Icaffe2/contrib/aten -I../third_party/onnx -Ithird_party/onnx -I../third_party/foxi -Ithird_party/foxi -I../caffe2/../torch/csrc/api -I../caffe2/../torch/csrc/api/include -I../caffe2/aten/src/TH -Icaffe2/aten/src/TH -I../caffe2/../torch/../aten/src -Icaffe2/aten/src -Icaffe2/../aten/src -Icaffe2/../aten/src/ATen -I../caffe2/../torch/csrc -I../caffe2/../torch/../third_party/miniz-2.0.8 -I../aten/src/TH -I../aten/../third_party/catch/single_include -I../aten/src/ATen/.. -Icaffe2/aten/src/ATen -I../third_party/miniz-2.0.8 -I../caffe2/core/nomnigraph/include -Icaffe2/aten/src/THC -I../aten/src/THC -I../aten/src/THCUNN -I../aten/src/ATen/cuda -I../c10/.. -Ithird_party/ideep/mkl-dnn/include -I../third_party/ideep/mkl-dnn/src/../include -I../third_party/QNNPACK/include -I../third_party/pthreadpool/include -I../aten/src/ATen/native/quantized/cpu/qnnpack/include -I../aten/src/ATen/native/quantized/cpu/qnnpack/src -I../third_party/QNNPACK/deps/clog/include -I../third_party/NNPACK/include -I../third_party/cpuinfo/include -I../third_party/fbgemm/include -I../third_party/fbgemm -I../third_party/fbgemm/third_party/asmjit/src -I../third_party/FP16/include -I../c10/cuda/../.. -isystem third_party/gloo -isystem ../cmake/../third_party/gloo -isystem ../cmake/../third_party/googletest/googlemock/include -isystem ../cmake/../third_party/googletest/googletest/include -isystem ../third_party/protobuf/src -isystem /Users/distiller/workspace/miniconda3/include -isystem ../third_party/gemmlowp -isystem ../third_party/neon2sse -isystem ../third_party -isystem ../cmake/../third_party/eigen -isystem /Users/distiller/workspace/miniconda3/include/python3.7m -isystem /Users/distiller/workspace/miniconda3/lib/python3.7/site-packages/numpy/core/include -isystem ../cmake/../third_party/pybind11/include -isystem /opt/rocm/hip/include -isystem /include -isystem ../cmake/../third_party/cub -isystem /Developer/NVIDIA/CUDA-9.2/include -isystem ../third_party/ideep/mkl-dnn/include -isystem ../third_party/ideep/include -isystem include -Wno-deprecated -fvisibility-inlines-hidden -Wno-deprecated-declarations -Xpreprocessor -fopenmp -I/usr/local/include -DUSE_FBGEMM -DUSE_QNNPACK -DUSE_PYTORCH_QNNPACK -O2 -fPIC -Wno-narrowing -Wall -Wextra -Wno-missing-field-initializers -Wno-type-limits -Wno-array-bounds -Wno-unknown-pragmas -Wno-sign-compare -Wno-unused-parameter -Wno-unused-variable -Wno-unused-function -Wno-unused-result -Wno-strict-overflow -Wno-strict-aliasing -Wno-error=deprecated-declarations -Wno-error=pedantic -Wno-error=redundant-decls -Wno-error=old-style-cast -Wno-invalid-partial-specialization -Wno-typedef-redefinition -Wno-unknown-warning-option -Wno-unused-private-field -Wno-inconsistent-missing-override -Wno-aligned-allocation-unavailable -Wno-c++14-extensions -Wno-constexpr-not-const -Wno-missing-braces -Qunused-arguments -fcolor-diagnostics -faligned-new -fno-math-errno -fno-trapping-math -Wno-unused-private-field -Wno-missing-braces -Wno-c++14-extensions -Wno-constexpr-not-const -DHAVE_AVX_CPU_DEFINITION -DHAVE_AVX2_CPU_DEFINITION -O3  -isysroot /Applications/Xcode-9.0.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.13.sdk -mmacosx-version-min=10.9 -fPIC   -DCAFFE2_USE_GLOO -DCUDA_HAS_FP16=1 -DHAVE_GCC_GET_CPUID -DUSE_AVX -DUSE_AVX2 -DTH_HAVE_THREAD -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wno-write-strings -Wno-unknown-pragmas -Wno-missing-braces -fvisibility=hidden -DCAFFE2_BUILD_MAIN_LIB -O2 -DASMJIT_STATIC -std=gnu++11 -O3  -mavx2 -mfma  -DCPU_CAPABILITY=AVX2 -DCPU_CAPABILITY_AVX2 -MD -MT caffe2/CMakeFiles/torch.dir/__/aten/src/ATen/native/cpu/MultinomialKernel.cpp.AVX2.cpp.o -MF caffe2/CMakeFiles/torch.dir/__/aten/src/ATen/native/cpu/MultinomialKernel.cpp.AVX2.cpp.o.d -o caffe2/CMakeFiles/torch.dir/__/aten/src/ATen/native/cpu/MultinomialKernel.cpp.AVX2.cpp.o -c aten/src/ATen/native/cpu/MultinomialKernel.cpp.AVX2.cpp
Nov 14 17:04:39 aten/src/ATen/native/cpu/MultinomialKernel.cpp.AVX2.cpp:45:19: error: no matching function for call to 'isfinite'
Nov 14 17:04:39       TORCH_CHECK(std::isfinite(val), "invalid multinomial distribution (encountering probability entry = infinity or NaN)");
Nov 14 17:04:39                   ^~~~~~~~~~~~~

@vishwakftw
Copy link
Contributor Author

@ezyang, this seems to me like a clang specific error. Do you have recommendations for a fix?

@ezyang
Copy link
Contributor

ezyang commented Nov 19, 2019

Well, I'm not too sure about this case specifically, but usually if a std:: function doesn't support all the overloads we need, we make our own version of it with all of the overloads as necessary. In this case I think the dumb way of making it work on clang is casting the half to float before running isfinite on it.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ezyang is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@vishwakftw vishwakftw deleted the multinomial-for-half branch November 20, 2019 21:09
@facebook-github-bot
Copy link
Contributor

@ezyang merged this pull request in ae6af8d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"multinomial_kernel_cuda" not implemented for 'Half'

4 participants