Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to use Gaia EDR3 when selecting GFAs #734

Merged
merged 14 commits into from May 25, 2021
Merged

Conversation

geordie666
Copy link
Contributor

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:

  • Initializing the SUBPRIORITY value in target files with better-constructed random seeds.
    • These seeds are now based on the HEALPixel number when parallelizing across pixels.
  • Documenting that the desitarget.io.write_* routines alter the value of SUBPRIORITY, and including a keyword option to turn that overwriting behavior off.
  • Adding a leq kwarg when reading ledgers with a specific isodate.
    • This keyword allows ledgers to be queried before-and-on that date, which supplements the default option of querying the ledgers strictly before the passed isodate.

@geordie666
Copy link
Contributor Author

@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
Copy link

coveralls commented May 19, 2021

Coverage Status

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

@geordie666
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Member

@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
Copy link
Member

@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
Copy link
Contributor Author

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

I'll merge this soon.

@geordie666 geordie666 merged commit 9faa019 into master May 25, 2021
@geordie666 geordie666 deleted the ADM-edr3-gfas branch May 25, 2021 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants