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

Duplicate KPNO extinction tables #8

Open
dkirkby opened this issue Feb 21, 2016 · 11 comments
Open

Duplicate KPNO extinction tables #8

dkirkby opened this issue Feb 21, 2016 · 11 comments
Assignees
Milestone

Comments

@dkirkby
Copy link
Member

dkirkby commented Feb 21, 2016

Why do we have two versions of the KPNO zenith extinction coefficients:

  1. data/spectra/ZenithExtinction-KPNO.dat covers 3500-10,000A in 0.1A steps with 7 digits of precision.
  2. data/sky/kpnoextinct_lunarmodel.dat covers 3200-10,600A in 1.0A steps with 6 digits of precision.

Both tables have identical values (except for the extra digit of precision) for the wavelengths they have in common.

@crockosi Is there a reason to prefer the lower-resolution, wider coverage file for your lunarmodel.pro code? I propose to use the higher-resolution file for desihub/specsim#9.

@sbailey
Copy link
Contributor

sbailey commented Feb 23, 2016

I suspect that Jeremy added the second one while implementing the lunar model simply because he was unaware of the first one, which was added by David (Schlegel) possibly because he was unaware of the original one in data/inputs/throughput/ZenithExtinction-KPNO.fits. That one is used by bin/combine_throughputs.py while creating the data/throughput/thru-[brz].fits files; these contain a 4th (and 5th and 6th) copy of the extinction in the "extinction" column of the binary table in HDU1.

There could be a reasonable argument for keeping an extinction column in the thru-[brz].fits files so that they can be one-stop-shopping, as long as that is algorithmically created by a canonical input version. And we do keep text and fits versions of some other files, so ZenithExtinction-KPNO.fits and ZenithExtinction-KPNO.dat aren't completely crazy other than being in different directories, which should be cleaned up. kpnoextinct_lunarmodel.dat appears to be the outlier.

@dkirkby
Copy link
Member Author

dkirkby commented Feb 23, 2016

There are more duplicates that I realized! What is the plan for files under desimodel/data/ going forward, now that the code lives in this github package? Are we are aiming for a slimmed down set of canonical files? Specsim is now flexible enough to use any of these sources, including a column from one of the thru-[brz].fits files.

@sbailey
Copy link
Contributor

sbailey commented Feb 23, 2016

The plan is for the code to be on github, while installing the data from svn. Even though the data is a bit of a mess right now, I would suggest waiting until after the reviews to do a cleanup so that we don't break currently working code. Cleanup could include:

  • removing duplicates
  • Makefile to convert inputs from DocDB into outputs to use
  • convert ascii tables to ecsv ascii to get units and other metadata in there
  • data/spectra directory contains both inputs and outputs. Consider Makefile style generation of outputs upon install, or at least separating what is an input (spec-.dat) vs. what is an output (sn.dat)
  • streamline code install such that "python setup.py install" would also install the matching data directory from svn.
  • write function to generate speclite desi config file, given desimodel input, and keep that in desimodel and use it from there. We should not rely upon a desi config file that replicates numbers from desimodel that are kept in sync by hand.

The last item could be done now since it is only an addition, not a rearrangement or deletion.

@moustakas
Copy link
Member

I would also like to retire some or all of the 'fiducial' spectra in (the SVN) desimodel/data/spectra directory and either replace these with median templates of the appropriate spectral type or build the template on-the-fly using desisim.

@dkirkby
Copy link
Member Author

dkirkby commented Feb 23, 2016

The spec_sim_ config file duplicates a subset of desi.yaml but also contains a lot of other info (including machine readable units for the duplicated values), so I don't think the last item is so straightforward. Would it be enough to have a script in the github desimodel that validates the specsim config against the authoritative desi.yaml? In any case, this task should probably also wait until after the reviews.

@weaverba137
Copy link
Member

@sbailey, I'm assigning this to you so that you can assign someone to resolve this issue. Clearly the more duplicate data we can remove, the faster it will be to install and test desimodel.

@weaverba137
Copy link
Member

Please clean up these duplicate files, because creating multiple data models for these files is absurd!

@weaverba137
Copy link
Member

Adding label 'data model' note that fixing this issue will also involve changes to the data model.

@weaverba137
Copy link
Member

There are actually three extinction files. In addition to those mentioned above, there is inputs/throughput/ZenithExtinction-KPNO.fits.

@weaverba137
Copy link
Member

At least one of these files is calibrated to air, not vacuum wavelengths. @julienguy , could you add your comments on other problems?

@weaverba137
Copy link
Member

@julienguy , a while ago you mentioned there were other problems with these extinction tables. Could you please take a look so we can decide which one to keep?

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

No branches or pull requests

4 participants