diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f6002efee1..4f0ec26a02 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -57,3 +57,4 @@ Bug Fixes - Changed the SED class to correctly broadcast over waves when the SED is constant. (#1228) - Fixed some errors when drawing ChromaticTransformation objects with photon shooting. (#1229) - Fixed the flux drawn by ChromaticConvolution with photon shooting when poisson_flux=True. (#1229) +- Fixed a slight inaccuracy in the FFT phase shifts for single-precision images. (#1231, #1234) diff --git a/src/SBTransform.cpp b/src/SBTransform.cpp index 7f01eaa253..03e87fc624 100644 --- a/src/SBTransform.cpp +++ b/src/SBTransform.cpp @@ -735,24 +735,26 @@ namespace galsim { // A helper function for filKImage below. // Probably not worth specializing using SSE, since not much time spent in this. + // Also, to avoid rounding errors accumulating, it's better to do this in double + // precision anyway, so we probably don't really want a single-precision SSE version. template - inline void fillphase_1d(std::complex* kit, int m, T k, T dk) + inline void fillphase_1d(std::complex* kit, int m, double k, double dk) { #if 0 // Original, more legible code for (; m; --m, k+=dk) - *kit++ = std::polar(T(1), -k); + *kit++ = std::polar(1., -k); #else // Implement by repeated multiplications by polar(1, -dk), rather than computing // the polar form each time. (slow trig!) // This is mildly unstable, so guard the magnitude by multiplying by // 1/|z|. Since z ~= 1, 1/|z| is very nearly = |z|^2^-1/2 ~= 1.5 - 0.5|z|^2. - std::complex kpol = std::polar(T(1), -k); - std::complex dkpol = std::polar(T(1), -dk); + std::complex kpol = std::polar(1., -k); + std::complex dkpol = std::polar(1., -dk); *kit++ = kpol; for (--m; m; --m) { - kpol = kpol * dkpol; - kpol = kpol * T(1.5 - 0.5 * std::norm(kpol)); + kpol *= dkpol; + kpol *= (1.5 - 0.5 * std::norm(kpol)); *kit++ = kpol; } #endif @@ -812,28 +814,25 @@ namespace galsim { dkyx *= ceny; // Only ever use these as sum of kx + ky, so add them together now. - T k0 = kx0 + ky0; - T dk0 = dkxy + dky; - T dk1 = dkx + dkyx; + double k0 = kx0 + ky0; + double dk0 = dkxy + dky; + double dk1 = dkx + dkyx; for (int j=n; j; --j, k0+=dk0, ptr+=skip) { - T k = k0; + double k = k0; #if 0 // Original, more legible code for (int i=m; i; --i, k+=dk1) { - *ptr++ *= std::polar(T(fluxScaling), -k); + *ptr++ *= std::polar(fluxScaling, -k); } #else // See comments above in fillphase_1d for what's going on here. - // MJ: Could consider putting this in the InnerLoop struct above and write - // specialized SSE versions, since native complex multiplication is terribly slow. - // But this use case is very rare, so probably not worth it. - std::complex kpol = std::polar(T(1), -k); - std::complex dkpol = std::polar(T(1), -dk1); + std::complex kpol = std::polar(1., -k); + std::complex dkpol = std::polar(1., -dk1); *ptr++ *= fluxScaling * kpol; for (int i=m-1; i; --i) { - kpol = kpol * dkpol; - kpol = kpol * T(1.5 - 0.5 * std::norm(kpol)); + kpol *= dkpol; + kpol *= (1.5 - 0.5 * std::norm(kpol)); *ptr++ *= fluxScaling * kpol; } #endif diff --git a/tests/test_gaussian.py b/tests/test_gaussian.py index 852ca24675..9c740c05c3 100644 --- a/tests/test_gaussian.py +++ b/tests/test_gaussian.py @@ -386,6 +386,26 @@ def test_ne(): check_all_diff(gals) +@timer +def test_accurate_shift(): + """Test that shifted Gaussian looks the same with real and fourier drawing. + """ + # This is in response to issue #1231 + + gal = galsim.Gaussian(sigma=1.).shift([5,5]).withFlux(200) + + real_im = gal.drawImage(nx=128, ny=128, scale=0.2, method='real_space') + fft_im = gal.drawImage(nx=128, ny=128, scale=0.2, method='fft') + print('max abs diff = ',np.max(np.abs(real_im.array - fft_im.array))) + np.testing.assert_allclose(fft_im.array, real_im.array, rtol=1.e-7, atol=1.e-7) + + # In double precision it's almost perfect. + real_im = gal.drawImage(nx=128, ny=128, scale=0.2, method='real_space', dtype=float) + fft_im = gal.drawImage(nx=128, ny=128, scale=0.2, method='fft', dtype=float) + print('max abs diff = ',np.max(np.abs(real_im.array - fft_im.array))) + np.testing.assert_allclose(fft_im.array, real_im.array, rtol=1.e-14, atol=1.e-14) + + if __name__ == "__main__": testfns = [v for k, v in vars().items() if k[:5] == 'test_' and callable(v)] for testfn in testfns: