search

GalSim-developers / GalSim

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

#1105 Fix convergence test for InvertAB #1106

Closed rmjarvis closed 3 years ago

rmjarvis commented 4 years ago

(#1105) The convergence test for InvertAB had used an absolute step size, rather than a relative one. If |x| or |y| is of order 10^4, this could spuriously claim that the calculation was not converged, when it actually was converged.

The fix is to use relative steps when |x| or |y| > 1.

rmjarvis commented 4 years ago

I think this change was really an error in our convergence criterion, so I think it should be very rare now for things to fail. I think probably only for positions that are way outside the region of applicability of the SIP or TPV coefficients. But I don't have a problem adding more detail to the error message, since then a user could catch the error and see whether they care about any non-convergence points.

rmjarvis commented 4 years ago

I added information to the error message about which indices failed. It will still fail to continue on with the rest of the radecToxy calculation, but at least the user can detect the problematic positions and redo without them. It would be significantly more work to have radecToxy finish the rest of the calculation and then keep track that it should raise an exception at the end. But if you think that's an important use case, we could consider doing that.

jmeyers314 commented 3 years ago

Thanks! This all looks good to me now.