Repository navigation
Fix ddof correction in variance over axes - #2938
Open
vera-rubin wants to merge 1 commit into
Open
vera-rubin wants to merge 1 commit into
vera-rubin wants to merge 1 commit into
Conversation
The divisor for an axis reduction was computed as (total_size - ddof) / output_size, so ddof was subtracted from the total element count before dividing by the number of output lanes. Divide first to get the per-lane sample count, then subtract ddof.
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Checklist
Description
xt::variance(e, axes, ddof)subtractedddoffrom the total element count before dividing by the number of output lanes, so the divisor was(size - ddof) / out_sizerather thansize / out_size - ddof. For a 2x3 array reduced over axis 1 withddof=2, this returned{4, 4}instead of{8, 8}.detail::mean_divisionnow takesddofseparately and subtracts it from the per-lane count.meanpassesddof = 0, so its results are unchanged. The existing assertion still checksddofagainst the total size. Behavior forddof >= Nremains unresolved, whereNis the sample count per lane. Forddof > N, the per-lane subtraction can underflow the unsigned divisor.Tested on Linux x86_64 in C++20 mode against
d9a57b696f42a815a40f08db5cb206d8960a9f4b, with xtl 0.8.0 atef84e9f27020ad54961c99acd293d80d4b775dd3.We added
xmath.variance_ddoftotest_xmath.cpp. It fails before the change and passes after. The fulltest_xtensor_libsuite passes with GCC 13.3, and the reproducer from the issue now matches NumPy with GCC 13.3 and Clang 18.1.Fixes #2937
— ChipLab · CLAB-2026-880