fix: bug in maxk algorithm for interpolated images#1356
fix: bug in maxk algorithm for interpolated images#1356
Conversation
|
@rmjarvis This one is ready for review! Getting the tests right was a pain since I don't have a great understand of how to call into the C++ layer properly. Hopefully the comments help explain what I am trying to do. I tried a bunch of versions where I remade new python-level interpolated images, tried comparing maxk for different image rotations/flips/etc. and none of those worked. The test here is a direct test of the details of the algorithm. |
beckermr
changed the title
| for offset in [3, 4, 5, 6, 7]: | ||
| im = galsim.Gaussian(fwhm=0.9 / 0.2).drawImage(scale=1) | ||
| iim = galsim.InterpolatedImage(im, scale=1) | ||
| orig_maxk = _compute_maxk_cpp(iim._xim, iim) |
There was a problem hiding this comment.
Why not just iim.maxk? I would have thought that would call the C++ code for you.
There was a problem hiding this comment.
That works for the initial maxk but doesn't come out right for the manipulated image. When I feed the new image back in, the maxk changes, but some how the fourier grid is different and so things don't match as I would expect.
There was a problem hiding this comment.
Here is what you get if you try and feed the new image into a new interpolated image object compared to the test I coded up:
| offset | orig | new via direct C++ | new via galsim II |
|---|---|---|---|
| 3 | 1.963495 | 2.159845 | 3.141593 |
| 4 | 1.963495 | 2.225295 | 3.141593 |
| 5 | 1.963495 | 2.290745 | 3.141593 |
| 6 | 1.963495 | 1.963495 | 3.141593 |
| 7 | 1.963495 | 1.963495 | 3.141593 |
The columns are:
- offset: the pixel offset
- orig: the original maxk value (same no matter how it is computed)
- new via direct C++: new maxk value computed via direct call into the C++ layer
- new via galsim II: new maxk value by feeding the new image back into an interpolated image
You can see it always returns pi which is really odd. :/
There was a problem hiding this comment.
I've pushed an update to the test suite that makes this table.
There was a problem hiding this comment.
I've also pushed some code that shows how the FFTs of the images relate given galsim's storage conventions.
2 participants
This PR fixes a small algorithmic bug in computing maxk for interpolated images.
I verified that the test failed without the bug fix.