add BGS templates and speclite & desitarget dependencies

[moustakas]

I'm creating this PR now for two reasons even though the code and templates are still in a state of flux/testing. The first reason is that BGS templates were promised to our Overlord by Valentine's day (i.e., today). And second, I want @crockosi to be able to move forward with #64 and not have to wait around for me any more.

In addition to adding a basic set of BGS templates (see #70) I have also updated the ELG, STAR, WD, and LRG templates in order to address #78 . In addition, the ELG templates (v1.5-->v2.0) now utilize Conroy's higher-resolution continuum models and the STAR models (v1.2-->v2.0) extend redward to 6 microns (thanks to Carlos!).

Consequently, once this PR is merged $DESI_BASIS_TEMPLATES should point to $DESI_ROOT/spectro/templates/basis_templates/v2.0

However, I still have to clean up some issues with the ELG and BGS templates, so user beware until everything is stable.

This PR also brings in an explicit dependence on the filter convolution stuff in (the master branch of) speclite (with many thanks to @dkirkby ). This dependence is still a bit fragile (e.g., the desisim.templates.QSO class isn't quite working until I can pad the spectra) and on the target selection cuts in desitarget.cuts.

I am also in the process of updating the emission-line spectrum priors in desisim.templates (which will also be part of this PR), so stand by for a bit more code. Updated TechNotes for all these spectral classes (including a new one for the BGS templates) are also forthcoming.

[sbailey]

Tests were failing due to a bad scipy install; it is unclear why that particular combination of astropy/scipy/numpy stopped working (welcome to dependency hell...), but it is the same symptom that was fixed by updating versions in desispec. I bumped the versions here and we'll see...

[sbailey]

Thanks for the BGS and other template updates, @moustakas. If the code and template updates will be stable by the end of this week, let's wait on merging this PR. In the meantime @crockosi can work off of a branch of this branch for the purposes of adding the BGS and bright flavors to newexp (#71) and we could merge them all in at the end of the week.

@dkirkby what is the timescale for speclite v0.3 with the updated config API (see #80)? Since this PR moves speclite to a core dependency instead of an isolated optional dependency, it would be preferable to do this after the API change so that we don't have to change again very soon.

For the record: I originally opposed making speclite a core dependency, just to avoid further dependency proliferation. However, I admit that the new filter code in speclite is a thing of beauty, and much better than any of the multiple versions of similar functionality that we've written in house, so it makes sense to depend on that. I withdraw my objection (as long as galsim doesn't become a core dependency of speclite...see comment thread in #77).

[sbailey]

Tests are failing due to:

File "/home/travis/build/desihub/desisim/py/desisim/templates.py", line 887, in make_templates
    decam_flux = self.decam.get_ab_maggies(flux, zwave, mask_invalid=True)
TypeError: get_ab_maggies() got an unexpected keyword argument 'mask_invalid'

speclite is installed in etc/travis_env_common.sh with "pip install speclite" without specifying a version, which presumably gets the latest registered tag. If "mask_invalid" is a feature in master that hasn't made it into a tag yet, can you cut a new tag @dkirkby? And/or @moustakas is there a reasonably simply way to work around using this keyword?

General guideline about dependencies: it is ok for us to temporarily depend upon master within our own desihub packages while working toward the next set of consistent tags (e.g. a feature added to desiutil to be used by desispec; once both are vetted together, a new tag for both). But for external products we should only depend upon tagged released versions, not master (speclite is admittedly a special case in the definition of "external", but we should still aim for only using tagged versions)

[dkirkby]

@sbailey The API change mentioned in #80 is for specsim, not speclite, so does not need to hold up this PR (confusing package names, I know!)

I totally agree that dependencies should be versioned and I will tag speclite v0.4 as soon as John and I converge on a few more details (desihub/speclite#12).

[dkirkby]

I am ready to tag speclite v0.4 once I get the green light from John.

@weaverba137 What is the correct way to specify a minimum version requirement for pip installation and travis testing?

[moustakas]

Apologies, yes, please feel free to tag.

[dkirkby]

I just tagged speclite v0.4 with improvements suggested by John (thanks!).

The travis tests are still failing, but not because of speclite version problems anymore (but I still think we want to specify the >= v0.4 requirement somewhere).

The failing tests are now test_random_seed and test_stars.

[weaverba137]

Pip: http://pip.readthedocs.org/en/stable/user_guide/#requirements-files
Travis: you'll have to edit the .travis.yml file and/or the travis_env_common.sh file.

[sbailey]

@moustakas it used to be the case that WD stars didn't have an FEH column in their metadata (and there was a test to confirm that). Now they do have an FEH column, but the entries are left as all zeros. Is that the intended behavior?

[sbailey]

@moustakas the ELG metadata WISE_FLUX has NaN values for w3 and w4 (columns 2 and 3). Is this intended? is NaN distinctly better than filling them in with 0 or -1, or perhaps not including those columns at all?

The random seed consistency test is failing on this since NaN != NaN. We can catch that as a special case in the tests if you really really want NaNs in there, but I thought I'd check first...

[dkirkby]

@moustakas Do you need W3, W4 magnitudes for any targets? If not, you could just remove these filters from your list, e.g.

my_filters = speclite.filters.load_filters('decam2014-*', 'wise2010-W1', 'wise2010-W2')

[moustakas]

I'll fix these tests (and apologies for not fixing them in the first place). A couple quick comments on this thread:

I should probably take (back) out the [Fe/H] metadata column (of zero value) for the white dwarfs, unless @crockosi weighs in and tells me/us otherwise.

Regarding the NaN's in WISE_FLUX, I don't know what we should put here. Maybe zero would be better (and 99 for the corresponding magnitudes, e.g., W1MAG, W2MAG, which are computed and stored simply for convenience). I added these columns so the metadata table would look more like a Tractor catalog. The idea would be to simulate a broad set of spectra without color cuts and then apply color cuts (via desitarget) after the fact (e.g., in order to explore redshift efficiency in other parts of color space).

But looking a bit more carefully at the target-selection code it looks like WISE_FLUX doesn't need to be a 4-element array, so I could just drop the W3/W4 photometry wholesale (as @dkirkby suggested, which I was aware of). Then the only undefined photometry (for now) would be the W1/W2 photometry of the QSOs and of the white dwarfs (all the other spectral models go red enough).

[crockosi]

No preference for having the [Fe/H] metadata for the WDs.

I could throw that question about useful metadata on the WDs to the MWS
working group, where there are a some experts, but for now it is probably
quickest to remove it.

--Connie

On Wed, Feb 17, 2016 at 3:54 PM, John Moustakas notifications@github.com
wrote:

I'll fix these tests (and apologies for not fixing them in the first
place). A couple quick comments on this thread:

I should probably take (back) out the [Fe/H] metadata column (of zero
value) for the white dwarfs, unless @crockosi
https://github.com/crockosi weighs in and tells me/us otherwise.

Regarding the NaN's in WISE_FLUX, I don't know what we should put here.
Maybe zero would be better (and 99 for the corresponding magnitudes, e.g.,
W1MAG, W2MAG, which are computed and stored simply for convenience). I
added these columns so the metadata table would look more like a Tractor
catalog. The idea would be to simulate a broad set of spectra without
color cuts and then apply color cuts (via desitarget) after the fact (e.g.,
in order to explore redshift efficiency in other parts of color space).

But looking a bit more carefully at the target-selection code it looks
like WISE_FLUX doesn't need to be a 4-element array, so I could just drop
the W3/W4 photometry wholesale (as @dkirkby https://github.com/dkirkby
suggested, which I was aware of). Then the only undefined photometry (for
now) would be the W1/W2 photometry of the QSOs and of the white dwarfs (all
the other spectral models go red enough).

—
Reply to this email directly or view it on GitHub
#81 (comment).

[moustakas]

OK, I think this PR is ready to merge. In addition to ~finalizing the ELG (v2.0) and BGS (v2.0) templates (currently transferring to NERSC...), this PR should also close #78, #70, and #8. It also fixes the issues identified above by @sbailey.

There have been some significant changes/updates to the templates but I've done as much spot-checking as I can do for now. All tests pass on my laptop (except the ones that depend on files I don't have...)

I still owe the team complete documentation (still in prep) but some high-level stuff is here:
https://github.com/desihub/desisim/blob/desisim-templates/doc/nb/calib-emline.ipynb
Thanks to @dkirkby for helping with the Gaussian mixture model stuff.

[sbailey]

Travis tests are failing, but I think that is due to out-of-date desisim-testdata templates. I can track this down tomorrow when I have a faster internet connection for moving templates around.

I'm also getting an unexpected success on flagging that QSO and stellar templates don't have WISE mags. Check and fix test if they do have WISE mags now.

+1 for using ecsv format for the data files.

Code looks good, but I want to get the Travis tests fully working with desisim-testdata before merging.

[moustakas]

The "expected failure" test on the wise photometry of the QSO and WD templates (note: the stars now go red enough) is now fixed.