Skip to content

MeanVarianceNormalization CPU EP axes attribute validation#11925

Merged
fdwr merged 2 commits intomasterfrom
user/dwayner/MeanVarianceNormalizationV9Validation
Jun 22, 2022
Merged

MeanVarianceNormalization CPU EP axes attribute validation#11925
fdwr merged 2 commits intomasterfrom
user/dwayner/MeanVarianceNormalizationV9Validation

Conversation

@fdwr
Copy link
Contributor

@fdwr fdwr commented Jun 21, 2022

Description: The CPU EP MVN kernel doesn't actually support the axes parameter (any arbitrary axes), which caused some test failures when comparing against the DML EP (which fully supports axes). Not supporting them is okay, but what's worse is that the code silently returns the wrong results for unsupported combinations, like HW and NC, which get mapped incorrectly to NHW and NCHW respectively. This change more robustly checks the axes parameter, rather than just checking for the presence of axis 1 without checking the other axes, given the CPU EP only supports 4D NCHW and NHW.

Motivation and Context

onnxruntime_test_all.exe
...
[==========] 3319 tests from 230 test suites ran. (372903 ms total)
[  PASSED  ] 3319 tests.

@hariharans29
Copy link
Member

Description: The CPU EP MVN kernel doesn't actually support the axes parameter (any arbitrary axes), which caused some test failures when comparing against the DML EP (which fully supports axes). Not supporting them is okay, but what's worse is that the code silent returns the wrong results for unsupported combinations, like HW and NC, which get mapped blindly and incorrectly to NCHW. This change more robustly checks the axes parameter, rather than just checking for the presence of axis 1 without checking the other axes, given the CPU EP only supports 4D NCHW and NHW.

Motivation and Context

onnxruntime_test_all.exe
...
[==========] 3319 tests from 230 test suites ran. (372903 ms total)
[  PASSED  ] 3319 tests.

Given the old logic, wouldn't HW be mapped to NHW and NC would get mapped to NCHW (as opposed to both getting mapped to NCHW) ?

@fdwr
Copy link
Contributor Author

fdwr commented Jun 22, 2022

@hariharans29 Correct - fixed PR description (I typed it correctly in my original issue :b).

@fdwr fdwr merged commit f6d2fe8 into master Jun 22, 2022
@fdwr fdwr deleted the user/dwayner/MeanVarianceNormalizationV9Validation branch June 22, 2022 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants