Honor the dtype argument of aten::mean when exporting without dim - #3009
Conversation
The no dim overload of aten::mean declares a dtype keyword in its schema, but the torchlib implementation did not accept it. Exporting torch.mean(x, dtype=torch.float64) for a float32 input succeeded silently, produced a FLOAT output and accumulated in float32. aten_mean is now a traced function that takes dtype and casts the input before ReduceMean, so the accumulation happens in the requested type exactly as PyTorch does. This differs on purpose from aten_mean_dim and aten_sum, which cast the reduced result. For the input 1e8, 1.0 and negative 1e8 the float32 mean is 0.0 while the float64 mean is one third, so casting after the reduction would still give the wrong value. aten_mean_complex gains the same argument and raises NotImplementedError when it is supplied, matching aten_mean_dim_complex. A new ops.aten.mean.dtype OpInfo exercises the fix, including a precision sensitive sample that fails when the cast is applied after the reduction. Fixes microsoft#3008
Promise Emmanuel Oluwadare (promiseeuler)
left a comment
There was a problem hiding this comment.
While comparing this with the duplicate implementation I had opened, I checked out commit c48d820 locally and verified the surviving approach directly. Ruff check and format check pass on all three changed files. Exporting torch.mean([1e8, 1, -1e8], dtype=torch.float64) produces Cast -> ReduceMean -> Squeeze, returns float64, and matches PyTorch at 1/3. The OpInfo coverage is stronger than a single e2e regression, and accepting dtype on the complex overload while explicitly rejecting unsupported conversion keeps its schema behavior consistent. This looks correct to me.
|
Thank you for the bug fix! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3009 +/- ##
=======================================
Coverage 72.63% 72.64%
=======================================
Files 265 265
Lines 32210 32214 +4
Branches 3042 3044 +2
=======================================
+ Hits 23397 23401 +4
Misses 7779 7779
Partials 1034 1034 ☔ View full report in Codecov by Harness. |
6e2292b
into
microsoft:main
|
Thanks for merging. Promise Emmanuel Oluwadare (@promiseeuler), sorry for the duplicate effort there, and thanks for checking mine out against your own rather than just letting it go. Verifying a competing implementation and then backing it is more than anyone would expect. If there are more torchlib lowerings worth going through, I am glad to keep picking them up. |
Honor the dtype argument of aten::mean when exporting without dim
Fixes #3008
Summary
The no dim overload of
aten::meandeclaresdtypein its schema (mean(Tensor self, *, ScalarType? dtype=None)), butaten_meaninonnxscript/function_libs/torch_lib/ops/core.pydid not accept the argument. Exportingtorch.mean(x, dtype=torch.float64)for a float32 input succeeded silently, emitted onlyReduceMean -> Squeeze, declared a FLOAT output, and accumulated in float32. For the input[[1e8, 1.0, -1e8]]PyTorch returns0.3333333333333333as float64 while the exported model returned0.0as float32.Changes
aten_meanis nowtrace_only=True, takesdtype: int = -1, and when a dtype is given castsselfbeforeReduceMean. The no dtype path is unchanged (ReduceMean -> Squeeze).aten_mean_complextakes the same argument and raisesNotImplementedErrorwhen it is supplied, matchingaten_mean_dim_complexandaten_sum_complex.ops.aten.mean.dtypeOpInfo intests/function_libs/torch_lib/extra_opinfo.py(sample_inputs_mean_dtype) registered againstcore_ops.aten_meaninops_test_data.py. It yieldsmake_tensorsamples of shapes(5, 5),(5,)and()plus the precision sensitive tensor[[1e8, 1.0, -1e8]], all withdtype=torch.float64.Why the cast happens before the reduction
aten_mean_dim(#2885) andaten_sumcast the reduced result after the reduction. That is not sufficient here: PyTorch accumulates in the requested dtype, so for[1e8, 1.0, -1e8]the float32 mean is0.0(the1.0is lost when added to1e8) while the float64 mean is1/3. Casting afterReduceMeanwould produce a DOUBLE tensor holding0.0, which still mismatches PyTorch. Casting the input first reproduces PyTorch semantics. The new test includes this sample specifically so that a cast after reduction implementation cannot pass by accident.Verification
Reporter's script (
torch.onnx.export(..., dynamo=True)oftorch.mean(x, dtype=torch.float64)with float32 input) on pristine main at a39c0a5:With this change:
pytest tests/function_libs/torch_lib/ops_test.py -k mean:ops_aten_mean_dtypesamples fail withTypeInferenceError: Inferred elem type differs from existing elem type: (1) vs (11)[[1e8, 1.0, -1e8]]sample fails:Expected 0.3333333333333333 but got 0.0The two additional skips in the fixed state are the function proto validity checks, which skip for traced functions.
ruff checkandruff format --check(ruff 0.15.1, the lintrunner pinned version) pass on the three changed files.Environment: Python 3.10, torch 2.13.0 (CPU), onnx 1.22.0, onnxruntime 1.23.2, macOS arm64.