desihub / desitarget

DESI Targeting
BSD 3-Clause "New" or "Revised" License
18 stars 23 forks source link

Add option to use Gaia EDR3 when selecting GFAs #734

Closed geordie666 closed 3 years ago

geordie666 commented 3 years ago

This PR is focused on updating the GFA targets to use Gaia EDR3.

It also adds a handful of requested features and bug fixes, which include:

geordie666 commented 3 years ago

@ameisner: This PR should update the GFAs to swap Gaia EDR3 information for DR2 information. The GFA files should now also include a GAIA_PHOT_G_N_OBS column.

I've created a full set of output files in /global/cscratch1/sd/adamyers/dr9/1.0.1.dev5024/gfas. It would be great if you could vet those files. There's no particular rush, as with Cori being down I won't merge this until Friday at the earliest.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.2%) to 59.337% when pulling 30322c8cd5b4b14968b05fdf928d3ccd877adf2e on ADM-edr3-gfas into 9f9d2a7ea2617e295c7b4b8453f442b39bcbaafd on master.

geordie666 commented 3 years ago

@ameisner: Before you check this, I just realized that for the sources that match the sweeps, I wound the coordinates back to a REF_EPOCH of 2015.5 to be consistent with the Legacy Surveys. But I didn't do that for the sources that are pure-Gaia-EDR3.

So, currently, in my test files, the Gaia-only sources are actually at a REF_EPOCH of 2016.0. I'll have to fix that, either by:

  1. updating the REF_EPOCH to be 2016.0 for the approprate sources; or
  2. shifting everything consistently to be at REF_EPOCH=2015.5.

Do you have a preference between 1. or 2.?

geordie666 commented 3 years ago

@ameisner: I have now aligned everything in the new GFA files to 2015.5 and updated the files in /global/cscratch1/sd/adamyers/dr9/1.0.1.dev5024/gfas accordingly. So, everything should be set to merge this PR and updated the GFA target files to Gaia EDR3.

Again, it would be excellent if you could check that those files meet your desiderata.

ameisner commented 3 years ago

@geordie666 thanks for all of these updates! Really sorry that I didn't see this pull request (GitHub notifications don't go to my e-mail inbox for some reason). I'll run some checks on your new GFA files later today / this evening.

ameisner commented 3 years ago

@geordie666 I ran a bunch of checks on your new eDR3 'gfas' files, and didn't find anything that I consider problematic. So these updates are good from my perspective. Looping over all 768 files, the two unique REF_CAT values were G3 and T2, and all G3 cases had REF_EPOCH = 2015.5. This all made sense to me. The one thing that caught my eye was that in the rare cases where REF_CAT = T2, GAIA_PHOT_G_MEAN_MAG is populated (apparently with Tycho V) even though GAIA_PHOT_G_N_OBS = 0. I can make sure that this won't be an issue for the fiberassign variability flagging I plan to do. I did also notice that this update to eDR3 made the 'gfas' catalogs such that Gaia placeholder values are always zeros, rather than a mixture of zeros and NaN's as in the 1.0.0 version of these files. That's also fine with me. Thanks again!

geordie666 commented 3 years ago

Thanks, Aaron. Yes, I subconsciously added the overwrite to fix the mix of NaNs and zeros!

I'll merge this soon.