Skip to content

Make idft give the same results before and after memoizing its factors - #34

Open
fdegros wants to merge 1 commit into
pjkundert:masterfrom
fdegros:fix-idft-memoization
Open

fdegros wants to merge 1 commit into
pjkundert:masterfrom
fdegros:fix-idft-memoization

Conversation

@fdegros

@fdegros fdegros commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #33.

Without numpy, ifft falls back to the pure-Python idft, which memoizes its multiplication factors. The first call accumulated its sum in a loop, while later calls used sum(), which since Python 3.12 uses compensated summation (and, since 3.14, for complex numbers too). So the same input gave results differing in the last bits, and test_dft_smoke (which checks that memoizing doesn't change the result) failed on Python 3.14 without numpy.

This computes both cases with the same loop (and the same division by (N+0j)), so the result no longer depends on whether the factors were already memoized. The first call's numbers are unchanged.

  • Change: slip39/recovery/entropy.py (idft).
  • Test: new test_idft_memoized in slip39/recovery_test.py, which calls idft directly, so it also fails on current master when numpy is installed, and passes with this change.
  • pytest slip39/recovery_test.py: 20 passed (with numpy). Without numpy, the whole suite (minus the invoice/communications tests, which need optional packages) passed too: 47 passed, 4 skipped.

Without numpy, ifft falls back to idft, which memoizes its multiplication factors.  The
first call accumulated its sum in a loop, while later calls used sum(), which since
Python 3.12 uses compensated summation (and, since 3.14, for complex numbers too), so
the same input gave results differing in the last bits, and test_dft_smoke failed.

Compute both cases with the same loop (and the same division by (N+0j)), so the result
no longer depends on whether the factors were already memoized.  Add test_idft_memoized,
which calls idft directly (so it also fails with numpy installed, without this change).

See pjkundert#33.

This branch has not been deployed

No deployments
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.

test_dft_smoke fails without numpy on Python 3.14: idft differs once its factors are memoized

1 participant