search

GalSim-developers / GalSim

The modular galaxy image simulation toolkit. Documentation:
http://galsim-developers.github.io/GalSim/
Other
223 stars 105 forks source link

#1265 quinticje #1266

Closed jecampagne closed 3 months ago

jecampagne commented 8 months ago

Hello, This PR is related to issue #1265 where I've announced a modified version of the default GalSim Quintic filter/kernel (Bernstein & Gruen 2014 paper). This new 5th-order filter is called "QuinticBis" in the PR and $K^{je}_5$ is the table below which is an update of Table 1 of the paper. image.

The files modified are

  1. in src: Interpolant.cpp
  2. in include: Interpolant.h
  3. pysrc: Interpolant.cpp
  4. interpolant.py to create de "QuinticBis" class following the Quintic one.

For the unit test, I have adapted the following functons in test_interpolatedimage.py

  1. test_interpolant
  2. test_unit_integral
  3. test_realspace_conv
  4. test_conserve_dc
  5. test_ii_shoots
  6. test_quintic_glagu
  7. test_depixelize and create test_QuinticBis_ref. One point: as Quintic is the default kernel for InterpolatedImage, I have NOT change this default, so many of the tests do not use "QuinticBis".

I have followed the README workflow, although I am not sure that PRs from non-official-GalSim-developper are really envisaged. Hope that I've not missed some important x-checks.

Let me know what do you think. Thanks

rmjarvis commented 5 months ago

Jean-Eric, I'm just checking on the status of this PR. There doesn't seem to be any progress since Christmas. At the time, my request was to describe what the pros and cons are of this interpolant relative to the normal Quintic. I see you added a bunch of very mathy differences. But I don't think a typical user would know what to do with those. What I was looking for was in-practice differences in the rendered images when this is used for k_interpolant in InterpolatedImage compared to the regular Quintic. E.g. the prominance of side lobes or the fidelity of the shear or size recovered from hsm.

Do you have any appetite for doing such investigations to give users appropriate guidance? If so, we can keep this PR open. But if not, I'm inclined to close it as unnecessary.

If you do want to keep it open, I'll note that the tests are failing. Also, it should be rebased onto main, not releases/2.5.

rmjarvis commented 3 months ago

OK, I'm assuming the lack of response means you've abandoned this effort. Feel free to reopen if you want to try to address the above points.