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

WISE mags for stdstars #263

Merged
merged 3 commits into from
Aug 24, 2016
Merged

WISE mags for stdstars #263

merged 3 commits into from
Aug 24, 2016

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Aug 19, 2016

We should have run the integration test prior to merging John's big template refactor desihub/desisim#132 . This PR fixes at least one issue: standard stars now have WISE magnitudes included, but our default stdstar template set /project/projectdirs/desi/spectro/templates/star_templates/v1.1/star_templates_v1.1.fits doesn't have sufficient wavelength coverage to calculate WISE mags to compare those templates with the data mags. Perhaps we need to update the template set?

This PR has a work around of simply not calculating the WISE magnitudes for these stdstar templates since we don't normalize to them anyway.

I'm still getting other random redshift failures, but it might be because the new templates are generating fainter ELGs that sometimes should fail.

i.e. don't merge yet, but I wanted to get @moustakas looking at this to see if we should update the templates that we use for stdstar comparisons, or do what I did here, or something else.

@moustakas
Copy link
Member

Just to remind everybody, we discussed some possibilities in #219.

I think we should revisit this question because I don't think we should be considering all the stellar templates as potential standard stars -- when we start generating datasets with more realistic systematic errors I think this is going to get our flux-calibration into trouble... The point is that we should have a semi-static, restricted set of templates that we consider potential standard-star SEDs.

But to the question on this thread, I think we should just update the starmodels option in desispec.pipeline.plan to point to the latest stellar models in ${DESI_ROOT}/spectro/templates/basis_templates/v2.2/star_templates_v2.1.fits. Is that acceptable?

Also, is the STD_TEMPLATES environment variable used anymore? It's mentioned in the documentation:

but isn't used in any code as far as I could tell.

@sbailey
Copy link
Contributor Author

sbailey commented Aug 24, 2016

Still a WIP, but two more updates so that the final integration test checks don't report false failures:

  • consider ELG [OII] flux when assigning ok/OOPS in the final integration test check
  • allow either STD or FSTD to be classified as a STAR

I also turned off the desi_qa_zfind in old_integration_test pending the fix to issue desihub/desisim#149

I have not yet tried using ${DESI_ROOT}/spectro/templates/basis_templates/v2.2/star_templates_v2.1.fits but that seems reasonable. The stdstar matching code already trims the templates down to those whose colors match the target (with some buffer to account for measurement error) so we aren't comparing to every template. I'm not sure we need to do this switch prior to merging this though; the remaining integration test failures turned out to be a combination of the above bookkeeping false failures and a fibermap MAG issue in desisim for which I will submit a PR shortly.

@moustakas moustakas merged commit 7ade9c2 into master Aug 24, 2016
@moustakas
Copy link
Member

Merging this now so we can proceed in other fronts, but I would like to revisit how the standard star templates are incorporated into the pipeline.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants