-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Fix CUDA ReduceSum erroring out on empty tensors with explicit axes #28353
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.
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
Changes from 7 commits
3e0e208
6732857
2c2003f
657664e
0b7616e
1cc286e
b82b4f1
ab891fc
7e84448
235ac37
5b8a5a4
db8739b
f00d512
bb43216
90c5b9f
5dbaaf9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -298,22 +298,17 @@ | |
| prepare_reduce_metadata.output_dims = input_shape.AsShapeVector(); | ||
| for (auto axis : axes) { | ||
| axis = HandleNegativeAxis(axis, rank); | ||
| ORT_ENFORCE(input_dims[axis] != 0, | ||
|
justinchuby marked this conversation as resolved.
|
||
| "Can't reduce on dim with value of 0 if 'keepdims' is false. " | ||
| "Invalid output shape would be produced. input_shape:", | ||
| input_shape); | ||
| prepare_reduce_metadata.output_dims[axis] = 1; | ||
| reduced[axis] = true; | ||
| } | ||
| } else { | ||
| // no axes provided (i.e.) default axes => reduce on all dims | ||
| // Each reduced dim becomes 1 (even if the original dim was 0 — the | ||
| // reduction collapses the axis regardless of its size). | ||
| prepare_reduce_metadata.output_dims.reserve(input_dims.size()); | ||
| for (auto dim : input_dims) { | ||
| ORT_ENFORCE(keepdims || dim != 0, | ||
| "Can't reduce on dim with value of 0 if 'keepdims' is false. " | ||
| "Invalid output shape would be produced. input_shape:", | ||
| input_shape); | ||
| prepare_reduce_metadata.output_dims.push_back(dim == 0 ? 0 : 1); | ||
| for (size_t i = 0; i < input_dims.size(); ++i) { | ||
| prepare_reduce_metadata.output_dims.push_back(1); | ||
| reduced[i] = true; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -377,7 +372,37 @@ | |
| auto& output_dims_cudnn = prepare_reduce_metadata.output_dims_cudnn; | ||
| // special case when there is a dim value of 0 in the shape. | ||
| if (input_count == 0) { | ||
| assert(output.Shape().Size() == 0); | ||
| // Empty input reduction: output may still be non-empty when only some | ||
| // axes are reduced. Per ONNX spec, fill with the reduction identity. | ||
| if (output_count > 0) { | ||
| // For types that don't support std::numeric_limits natively (MLFloat16, | ||
| // BFloat16), use float intermediary and convert via CudaT. | ||
| if (cudnn_reduce_op == CUDNN_REDUCE_TENSOR_AVG) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CPU and CUDA now disagree on empty-set ReduceMean, and the new generic tests encode only the CUDA behavior. The CPU empty-input fast path still routes through reduction_ops.cc:924, ReduceAggregatorMean still inherits ReduceAggregatorSum at reduction_ops.h:307, and the inherited empty-set fill is still 0 at reduction_ops.h:224. CUDA was changed to emit quiet_NaN() for empty ReduceMean at reduction_ops.cc:380, reduction_ops.cc:381, reduction_ops.cc:816, and reduction_ops.cc:817, and the new test expects NaN at reduction_ops_test.cc:6512. ONNX says empty-set ReduceMean is undefined, so choosing NaN is defensible, but this PR has not made ORT internally consistent. As written, the “tests for all EPs” claim is not satisfactorily addressed since it should fail on CPU. This needs to be consistent and the test should cover it. Please, consider using zeros from historical perspective.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in 7e84448. CPU and CUDA are now consistent — both fill ReduceMean empty-set with 0:
Test |
||
| // ReduceMean on empty set is undefined (0/0). Fill with 0. | ||
| CUDA_RETURN_IF_ERROR(cudaMemsetAsync(output.MutableDataRaw(), 0, | ||
| output.SizeInBytes(), stream)); | ||
| } else if (cudnn_reduce_op == CUDNN_REDUCE_TENSOR_MUL) { | ||
| // ReduceProd identity is 1. | ||
| CudaT one_val = ToCudaType<T>::FromFloat(1.0f); | ||
| std::vector<CudaT> ones(output_count, one_val); | ||
| CUDA_RETURN_IF_ERROR(cudaMemcpyAsync(output.MutableDataRaw(), ones.data(), | ||
| output.SizeInBytes(), cudaMemcpyHostToDevice, stream)); | ||
| } else if (cudnn_reduce_op == CUDNN_REDUCE_TENSOR_MIN) { | ||
| CudaT inf_val = ToCudaType<T>::FromFloat(std::numeric_limits<float>::infinity()); | ||
| std::vector<CudaT> vals(output_count, inf_val); | ||
| CUDA_RETURN_IF_ERROR(cudaMemcpyAsync(output.MutableDataRaw(), vals.data(), | ||
| output.SizeInBytes(), cudaMemcpyHostToDevice, stream)); | ||
| } else if (cudnn_reduce_op == CUDNN_REDUCE_TENSOR_MAX) { | ||
| CudaT neg_inf_val = ToCudaType<T>::FromFloat(-std::numeric_limits<float>::infinity()); | ||
| std::vector<CudaT> vals(output_count, neg_inf_val); | ||
| CUDA_RETURN_IF_ERROR(cudaMemcpyAsync(output.MutableDataRaw(), vals.data(), | ||
| output.SizeInBytes(), cudaMemcpyHostToDevice, stream)); | ||
| } else { | ||
| // Sum, SumSquare, L1, L2: identity is 0. | ||
| CUDA_RETURN_IF_ERROR(cudaMemsetAsync(output.MutableDataRaw(), 0, | ||
| output.SizeInBytes(), stream)); | ||
| } | ||
| } | ||
| return Status::OK(); | ||
| } | ||
|
|
||
|
|
@@ -770,7 +795,34 @@ | |
| auto& output_dims_cudnn = prepare_reduce_metadata.output_dims_cudnn; \ | ||
| \ | ||
| if (input_count == 0) { \ | ||
| assert(Y->Shape().Size() == 0); \ | ||
| /* Empty input reduction: fill output with the reduction identity. */ \ | ||
| /* ONNX spec: Sum→0, Prod→1, Min→+inf, Max→-inf, Mean→0. */ \ | ||
| if (Y->Shape().Size() > 0) { \ | ||
| typedef typename ToCudaType<T>::MappedType CudaT_local; \ | ||
| if (cudnn_reduce_op == CUDNN_REDUCE_TENSOR_MUL) { \ | ||
| /* Identity is 1 for product */ \ | ||
| CudaT_local one_val = ToCudaType<T>::FromFloat(1.0f); \ | ||
| std::vector<CudaT_local> ones(Y->Shape().Size(), one_val); \ | ||
| CUDA_RETURN_IF_ERROR(cudaMemcpyAsync(Y->MutableDataRaw(), ones.data(), \ | ||
| Y->SizeInBytes(), cudaMemcpyHostToDevice, Stream(ctx))); \ | ||
| } else if (cudnn_reduce_op == CUDNN_REDUCE_TENSOR_MIN) { \ | ||
| /* ONNX spec: "yields plus infinity (if supported) or max value" */ \ | ||
| CudaT_local inf_val = ToCudaType<T>::FromFloat(std::numeric_limits<float>::infinity()); \ | ||
| std::vector<CudaT_local> vals(Y->Shape().Size(), inf_val); \ | ||
| CUDA_RETURN_IF_ERROR(cudaMemcpyAsync(Y->MutableDataRaw(), vals.data(), \ | ||
| Y->SizeInBytes(), cudaMemcpyHostToDevice, Stream(ctx))); \ | ||
| } else if (cudnn_reduce_op == CUDNN_REDUCE_TENSOR_MAX) { \ | ||
| /* ONNX spec: "yields minus infinity (if supported) or minimum value" */ \ | ||
| CudaT_local neg_inf_val = ToCudaType<T>::FromFloat(-std::numeric_limits<float>::infinity()); \ | ||
| std::vector<CudaT_local> vals(Y->Shape().Size(), neg_inf_val); \ | ||
|
Check warning on line 817 in onnxruntime/core/providers/cuda/reduction/reduction_ops.cc
|
||
| CUDA_RETURN_IF_ERROR(cudaMemcpyAsync(Y->MutableDataRaw(), vals.data(), \ | ||
| Y->SizeInBytes(), cudaMemcpyHostToDevice, Stream(ctx))); \ | ||
| } else { \ | ||
| /* Sum, SumSquare, Mean, L1, L2, Amax: identity is 0 */ \ | ||
| CUDA_RETURN_IF_ERROR(cudaMemsetAsync(Y->MutableDataRaw(), 0, \ | ||
| Y->SizeInBytes(), Stream(ctx))); \ | ||
| } \ | ||
| } \ | ||
| return Status::OK(); \ | ||
| } \ | ||
| \ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.