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

Compressed stellar SV target classes #636

Merged
merged 22 commits into from Dec 28, 2020
Merged

Compressed stellar SV target classes #636

merged 22 commits into from Dec 28, 2020

Conversation

apcooper
Copy link
Contributor

@apcooper apcooper commented Aug 7, 2020

This is still under discussion, but here's a start on compressing the number of secondary bits used for stellar SV (MWS + Backup program).

Related info here: https://desi.lbl.gov/trac/wiki/MilkyWayWG/CommishSV#a2.4Targetselection (still pending approval from MWS).

The corresponding secondary indata files are here:
/global/homes/a/apcooper/mycfs/ssvtargets/combined/indata and /global/homes/a/apcooper/mycfs/ssvtargets/combined/docs

The docs are fairly basic. That can be fixed when the dust has settled on which targets are included as secondaries.

Related: @geordie666 may add BHB and RR Lyr to sv1_cuts directly. Currently these are secondary targets under MWS_MAIN_TRACER.

@geordie666
Copy link
Contributor

Thanks. Feel free to leave this as a WIP as long as you feel is needed. I don't think there's any particularly urgency, here (although I really appreciate the effort to merge bits). I guess it would be useful to finalize the set of bits before the deadline of the secondary call.

One note: Let's leave the RR Lyr as a "protected" secondary class for SV (as the work has already been done). I'll work to make them a special-case primary class for the Main Survey.

@apcooper
Copy link
Contributor Author

OK, thanks, I'll keep the RR Lyr separate.

@apcooper
Copy link
Contributor Author

@geordie666, @segasai;

I've attempted to code Sergey's BHB selection into desitarget.sv1_cuts for the stellar SV bright BHB sample:

def isMWS_bhb(primary=None, objtype=None,

Sergey, I'll need your help to check, and in particular make sure that the colour cuts are OK (and also what's going on with the WISE magnitude). Note that the gflux, rflux etc. have already been extinction-corrected outside this routine.

I tried to copy the logic in the SQL selection Sergey did a while ago. I also looked at the faint BHB selection Sergey submitted as a secondary target proposal recently (obviously I didn't copy that, I've no idea if those cuts are the same for fainter BHBs as brighter ones).

Anyway, @segasai, best if you can take a look when you have time. Thanks!

Some of the MWS secondary classes could be extended into very bright
conditions, at the cost of secondary bits. Not essential, small
numbers of targets.
@apcooper
Copy link
Contributor Author

apcooper commented Nov 22, 2020

6 remaining MWS secondary bits now as follows:

  • MWS_CALIB by prior agreement / one-line secondary proposal
  • MWS_WD_SV if we use the preselected catalog for WDs rather than the algorithm, need to dig out our conclusion with Boris on this.
  • MWS_MAIN_CLUSTER_SV folds together all the 'regular bright stellar SV' preselected members for SV-targeted open clusters, Globular clusters and dwarf galaxies into one bit. The fainter extension of these selections is the subject of a dark-time secondary proposal.
  • MWS_MAIN_RRLYR by prior agreement
  • MWS_NEARBY_OC_FILLER_SV to fill fibers on off-footprint open clusters (Hyades) with no LS data, but could be a less selective non-secondary draw from the same Gaia pool as the backup targets.
  • BACKUP_CALIB is the same as MWS_CALIB but for backup conditions (APOGEE stars etc.).

@apcooper
Copy link
Contributor Author

Reminder to self, we need to get rid of ALLMASK cuts if they now include the globular cluster mask...

@geordie666
Copy link
Contributor

The default "geometric mask cuts" (and a handy function to implement such cuts) are now here:

def imaging_mask(maskbits, bitnamelist=["BRIGHT", "GALAXY", "CLUSTER"]):

I don't think I called that new imaging_mask function anywhere in the MWS cuts for either SV or the Main Survey. It's definitely worth checking my working, though.

@apcooper
Copy link
Contributor Author

From Sergey:

first there is a
typo
rmz = rmag-zma
-> zmag

I also didn't notice any corrections for xtinction there ?
Also please change the bhb_sel range to -0.05,0.05 like here

(bhb_sel >= -0.05) & (bhb_sel <= 0.05)

@apcooper
Copy link
Contributor Author

Notes to self (and Sergey):

  • gflux, rflux, zflux, w1flux have all been corrected for extinction prior to this routine.

Replacing MWS_WD_SV (secondary target scheme) with MWS_WD (algorithmic
version).
@apcooper
Copy link
Contributor Author

apcooper commented Dec 20, 2020

Regarding WDs, see [desi-milkyway 1850].

This PR restores the algorithmic selection using in earlier runs.

Previously MWS was planning to introduce the bulk of MWS science WDs (16<r<20) via a secondary file (MWS_WD_SV). Faint science WDs for dark time are now the subject of a secondary target proposal.

@apcooper
Copy link
Contributor Author

apcooper commented Dec 20, 2020

4 remaining MWS primary SV program uses of secondary bits now:

  • MWS_CALIB by prior agreement / one-line secondary proposal
  • BACKUP_CALIB is the same as MWS_CALIB but for backup conditions (APOGEE stars etc.).
  • MWS_MAIN_CLUSTER_SV folds together all the 'regular bright stellar SV' preselected members for SV-targeted open clusters, Globular clusters and dwarf galaxies into one bit. The fainter extension of these selections is the subject of a dark-time secondary proposal.
  • MWS_MAIN_RRLYR by prior agreement

This PR now also eliminates

  • MWS_NEARBY_OC_FILLER_SV -- just not targeting the Hyades now, too complciated
  • MWS_WD_SV -- gone back to old algorithmic selection for SV

@apcooper apcooper marked this pull request as ready for review December 20, 2020 10:10
@apcooper
Copy link
Contributor Author

@geordie666, this is ready for a look with the hope it can be merged before SV proper, since it includes changes that are important for our SV program.

I can sort out the catalogs for MWS_CALIB, BACKUP_CALIB, MWS_MAIN_CLUSTER_SV and MWS_MAIN_RRLYR at NERSC (they are all there already, they just need to be put in the right place).

@apcooper
Copy link
Contributor Author

Catalogs are in:

/global/cfs/cdirs/desi/users/apcooper/ssvtargets/combined/

Copy link
Contributor

@geordie666 geordie666 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@apcooper: This is really careful work, as always. Thanks. I tested the code, and it seems to work just fine.

I have a couple of minor requests for changes regarding documentation and catching a divide-by-zero warning before it happens. Please do remember to update the SV wiki with Sergey's new BHB selection.

I'm happy to merge this when you're ready.

mws &= (bhb_sel >= -0.05) & (bhb_sel <= 0.05)

# APC back out the WISE error = 1/sqrt(ivar) from the SNR = flux*sqrt(ivar)
w1fluxerr = w1flux/w1snr
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appear to be some w1snr=0 cases, here. Is that really the desired behavior? You do appear to catch these further down in the code (I think) but it would be better not to flag divide-by-zero warnings in the first place. The solution might be as simple as setting any values with w1snr of 0 to 1e-7 instead, but catching that here instead of on the next line of code.

-----
- Criteria supplied by Sergey Koposov
- gflux, rflux, zflux, w1flux have been corrected for extinction
(unlike other MWS selections, which use obs_flux).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add an extra note here indicating the current version as compared to the wiki? So, something like:

    - Current version (12/21/20) is version 148 on `the SV wiki`_.

@geordie666
Copy link
Contributor

@apcooper: I really want to merge this so I can incorporate it into my current code development for secondary targets. So, I'm going to ignore my own minor requests for changes and move on.

Please, though, do update the SV wiki to reflect the changes in this PR.

@geordie666 geordie666 merged commit 426e0f2 into master Dec 28, 2020
@geordie666 geordie666 deleted the ssv_compress_bits branch December 28, 2020 20:22
@apcooper
Copy link
Contributor Author

@geordie666 many thanks, sorry I was slow on the uptake here over xmas. I've updated the wiki accordingly.

geordie666 added a commit that referenced this pull request Dec 30, 2020
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

2 participants