From e95430f456ce79ace32bbe78c1b949cd1d8f13c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Degros?= <57880064+fdegros@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:17:40 +1100 Subject: [PATCH] Make idft give the same results before and after memoizing its factors 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 #33. --- slip39/recovery/entropy.py | 35 +++++++++++++++-------------------- slip39/recovery_test.py | 17 ++++++++++++++++- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/slip39/recovery/entropy.py b/slip39/recovery/entropy.py index 1d376ef7..6f6dbffb 100644 --- a/slip39/recovery/entropy.py +++ b/slip39/recovery/entropy.py @@ -99,28 +99,23 @@ def idft( y: List[complex] ) -> List[complex]: """ N = len( y ) #assert N <= 170, f"idft: {N=} too large." # 512-bit entropy as 3-bit symbols - e_sk_N = idft.e_sk.setdefault( N, [] ) - if e_sk_N: - e_sk_N_i = iter( e_sk_N ) - result = [ - sum( - y_k * e_sk - for y_k,e_sk in zip( y, e_sk_N_i ) - ) / N+0j + e_sk_N = idft.e_sk.get( N ) + if e_sk_N is None: + #print( f"idft: Memoizing {N=} multiplication factors" ) + e_sk_N = idft.e_sk[N] = [ + cmath.exp( 2j * cmath.pi * n / N * k ) for n in range( N ) + for k in range( N ) ] - else: - #print( f"idft: Memoizing {N=} multiplication factors" ) - result = [] - for n in range( N ): - r = 0 - s = 2j * cmath.pi * n / N - for k in range( N ): - e_sk = cmath.exp( s * k ) - e_sk_N.append( e_sk ) - r += y[k] * e_sk - r /= N+0j - result.append( r ) + # Always accumulate in the same order (not w/ sum(), which since Python 3.12 uses compensated + # summation, and would give slightly different results than w/ the same code before and after + # the factors are memoized). + result = [] + for n in range( N ): + r = 0 + for k in range( N ): + r += y[k] * e_sk_N[n * N + k] + result.append( r / ( N+0j )) return result idft.e_sk = {} # noqa: E305 diff --git a/slip39/recovery_test.py b/slip39/recovery_test.py index a60e34ac..52aa3243 100644 --- a/slip39/recovery_test.py +++ b/slip39/recovery_test.py @@ -20,7 +20,7 @@ from .api import create, account, path_hardened from .recovery import recover, recover_bip39, shannon_entropy, signal_entropy, analyze_entropy -from .recovery.entropy import fft, ifft, pfft, dft, dft_on_real, dft_to_rms_mags, entropy_bin_dfts, denoise_mags, signal_draw, signal_recover_real, scan_entropy +from .recovery.entropy import fft, ifft, pfft, dft, idft, dft_on_real, dft_to_rms_mags, entropy_bin_dfts, denoise_mags, signal_draw, signal_recover_real, scan_entropy from .dependency_test import substitute, nonrandom_bytes, SEED_XMAS, SEED_ONES, SEED_ZERO from .util import avg, rms, ordinal, commas, round_onto @@ -1127,6 +1127,21 @@ def test_util(): assert round_onto( None, [-5, -1, None, 5], keep_sign=True ) is None +def test_idft_memoized(): + """The pure-Python idft (used by ifft when numpy isn't available) must give exactly the same + results before and after its multiplication factors are memoized. + + """ + x = [ 2, 3, 5, 7, 11 ] + y = dft( x ) + idft.e_sk.pop( len( y ), None ) # Ensure the factors aren't memoized yet + first = idft( y ) + assert len( idft.e_sk[len( y )] ) == len( y ) ** 2 # and now they are + second = idft( y ) + assert first == second + assert [ f.real for f in first ] == pytest.approx( x ) + + def test_dft_smoke(): """Test some basic assumptions on DFTs""" print()