desihub / desisurvey

Code for desi survey planning and implementation
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

DESI tiling dither pattern and QA code #87

Closed schlafly closed 6 years ago

schlafly commented 6 years ago

tileqa.qa() shows how to generate the new tiling pattern and demonstrates using the rest of the code to make QA plots.

This code had a number of dependences on routines that are in my usual library. I've copied them into this bit of code to make it stand-alone, but presumably some subset of the match2d, lb2uv, lb2tp, tp2uv, match_radec, gc_dist, and subslices routines have more typical DESI equivalents that don't need to be replicated here.

dkirkby commented 6 years ago

Since this generates and supports files in desimodel and has no desisurvey dependencies (but does depend on desimodel.focalplane), I think this belongs more naturally in the desimodel git package. @sbailey What was your rationale for adding it here?

sbailey commented 6 years ago

The code could go in either desisurvey or desimodel (though the final tile file needs to go in desimodel). I suggested putting it in desisurvey as part of QA of a tiling solution. So far desimodel is a collection of data files and lightweight utilities for using them; for the most part it is not a generator of data (other than reformatting from docdb) nor QA of those files like this code is. Code that you (@dkirkby) contributed would be the exceptions to that (desimodel.weather and desimodel.focalplane.sim). But it could be fine to expand desimodel in that direction if you feel strongly about it.

sbailey commented 6 years ago

Other code comments:

I like how this is self contained. Although match2d, lb2uv, lb2tp, tp2uv, etc. do have some code duplication with other routines, I don't think it is worth delaying this to try to change that (and thus complicate this).

tileqa.qa() mixes the generation of a specific tiling via logradecoffscheme() with the QA of that model. I'd prefer a small update to separate these such that tileqa.qa(ra, dec) could be used to evaluate any tiling.

It looks like we still need code for fully fleshing out the EBV_MED etc columns for a new tiling pattern.

While we're updating the tiling, there was a request awhile back to make the gray layer pass 0 instead of pass 4 to help humans remember it. That will probably break some code somewhere, but that code also shouldn't have hard coded what pass numbers mean what (it should use 'PROGRAM' column instead).

And then the tiles update complications continue: discussions during mock observing suggested that having PROGRAM='dark' (dark time survey science exposures) is confusing compared to FLAVOR='dark' (a calibration exposure with the shutter closed). Proposal was to change the PROGRAM to 'desi-dark' , 'desi-gray', 'desi-bright' instead of just dark/gray/bright.

Neither of these last two items should block getting this code in, but we should make a choice prior to updating the desi-tiles.fits file in desimodel itself.

schlafly commented 6 years ago

tileqa.qa() mixes the generation of a specific tiling via logradecoffscheme() with the QA of that model. I'd prefer a small update to separate these such that tileqa.qa(ra, dec) could be used to evaluate any tiling.

I had treated this primarily as a desire to get some working code in that others looking to improve the tiling scheme in the future could work from. If we think people will try to refine the scheme in the future, I can update it to be more generic... I'd probably add a __main__ that would wrap a more generic qa(...); the new __main__ would do what qa() does now.

It looks like we still need code for fully fleshing out the EBV_MED etc columns for a new tiling pattern.

Yes, I haven't done this. I'm hoping David uses his existing code to do this from the tile centers. I could write code for this, but the star_density column would require bringing in dependencies on 2MASS, which I don't really want to do.

Re names of passes... it is worth thinking about this. In contrast to the original tile file, all dithers are not strictly equal any more. I specifically optimized the first three passes to, combined, have little zero-pass coverage; something like the bright survey should definitely use passes (0, 1, 2) or (5, 6, 7). I think I'd recommend something like (0, 1, 2, 3) dark, (4) gray, (5, 6, 7) bright. We can, of course, reorder.

moustakas commented 6 years ago

The dependence on the dust maps is being tracked here and I know that @geordie666 is also working on this from off-list conversations -- https://github.com/desihub/desispec/issues/513

For the stellar density can we just use the Gaia chunks we use for the imaging?

sbailey commented 6 years ago

In the interests of not going further down the path of "perfect is the enemy of the good", I'm going to merge this now since it represents useful code used to generate a new tiling pattern in desimodel. Further updates, refactors, features, etc. can be handled by other PRs as needed. Thanks @schlafly.