LSSTDESC / imSim

GalSim based Rubin Observatory image simulation package
https://lsstdesc.org/imSim
BSD 3-Clause "New" or "Revised" License
36 stars 15 forks source link

Bad ra filtering for instcats #434

Closed jmeyers314 closed 6 months ago

jmeyers314 commented 9 months ago

@jbkalmbach has encountered an error where some objects from his instance catalog are not showing up in the generated images. I've tracked this down to bad ra/dec filtering here. Essentially, the min and max ra for the sensor in question are determined to be ~ [-0.16, +0.16] (degrees), and Bryce has an object with ra ~ 359.99. With angle wrapping, it should be in the "allowed" region, but because the code is too simple, it gets rejected.

There might be ways to rig the code to work in this particular instance, where the boresight is at (0, 0), but it's hard for me to imagine something effective near the pole (where any ra should be allowed). So I propose just eliminating this conditional entirely.

rmjarvis commented 9 months ago

There is a wrap function in our Angle class, which can be used for this exact use case. http://galsim-developers.github.io/GalSim/_build/html/units.html#galsim.Angle.wrap E.g.

min_ra = min_ra.wrap(cen_ra)
max_ra = max_ra.wrap(cen_ra)
[...]
if min_ra <= ra.wrap(cen_ra) <= max_ra:
    ...
jmeyers314 commented 9 months ago

Sure. But that doesn't solve the "sensor covers the pole" case I think.

rmjarvis commented 9 months ago

True. A focal plane covering the pole would be tricky to get right. We could always test center.distanceTo(sky_pos), but that would be much slower, since it uses trig functions.

rmjarvis commented 9 months ago

BTW, eliminating the test entirely will cause large slow downs in many cases I think. I believe it was put in because instance catalogs had objects for the whole focal plane, so for any particular CCD, it was spending lots of time discovering that objects were not on the image.

jmeyers314 commented 9 months ago

Hmm.. okay. Maybe add a branch to get_radec_limits. If any corner is within threshold of the pole, then keep the whole pole:

threshold=0.5 # deg
if max(abs([min_dec, max_dec])) > 90-threshold:
    if min_dec < 0:
        min_dec = -90
    else:
        max_dec = 90

That'll break in weird ways at some point, but 0.5 degree threshold is probably good enough for existing things we care to simulate.

jmeyers314 commented 9 months ago

https://github.com/LSSTDESC/imSim/pull/435

jchiang87 commented 6 months ago

Fixed by #435