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

Host authoritative specsim default configuration file #10

Open
dkirkby opened this issue Mar 7, 2016 · 7 comments
Open

Host authoritative specsim default configuration file #10

dkirkby opened this issue Mar 7, 2016 · 7 comments
Assignees
Milestone

Comments

@dkirkby
Copy link
Member

dkirkby commented Mar 7, 2016

The default DESI configuration used by specsim is currently specsim/data/config/desi.yaml. This issue is to host the authoritative version of this file in this package, and have the desisim quickgen and quickbrick scripts use this authoritative version by default.

In order to make this file authoritative, we also need to decide how to handle the 14 constants listed here:

http://specsim.readthedocs.org/en/stable/config.html#desi-configuration

These values are given in the specsim config file but are also under change control in DESI-doc-347.

An obvious question is why specsim doesn't just read desi.yaml from desimodel. The answer is that this file is incomplete (it does not have machine readable units and does not specify the additional files needed for simulation with authoritative throughputs, etc) and is not generalizable to other instruments. Specsim is designed to simulate any fiber spectrograph so needs a flexible, complete, and self-contained configuration mechanism.

A strawman solution is to provide a function get_specsim_config() in this package that returns a YAML file in the format that specsim expects and also validates that it is consistent with all quantities under change control specified by desi.yaml. The quickgen and quickbrick scripts would then use this function to retrieve their default specsim config. Comments?

@dkirkby
Copy link
Member Author

dkirkby commented Mar 7, 2016

For background on this issue see desihub/desisim#93.

@sbailey
Copy link
Contributor

sbailey commented Mar 7, 2016

I like get_specsim_config(), but should it return a yaml file, or the contents that would have been loaded from said yaml file? I think the latter is cleaner if that can be made compatible with specsim, since it doesn't involve making a temporary file just so that specsim can read it back in.

I had originally been thinking that we would provide a script in desimodel to convert its contents (including desimodel/data/desi.yaml) into a specsim_desi.yaml file that would also be kept in desimodel. But that requires humans to remember to update specsim_desi.yaml when desi.yaml is updated. Fragile. Doing it on the fly with get_specsim_config() is nice, at the cost of not having a file directly in github that is already the canonical file to use.

@weaverba137
Copy link
Member

@dkirkby, I'm assigning this to you so that you can assign it to someone who can resolve this issue.

@rainwoodman
Copy link
Contributor

what about returning a dict instead of a file? It sounds strange to serialize and deserialize; plus the load_config API can't handle string input anyways.

import desimodel
from specsim.config import Configuration
specsimconfig = Configuration(desimodel.get_specsim_config())
...
...

@weaverba137
Copy link
Member

Can this be resolved so that we can create (or not create) a data model for the file(s)?

@weaverba137 weaverba137 added this to the DC2017 Prep milestone May 4, 2017
@dkirkby
Copy link
Member Author

dkirkby commented May 5, 2017

I can take care of this but I'm not sure if this is still the direction we want to go. @sbailey ?

@weaverba137
Copy link
Member

We do want to move the filter curves over to desispec, but this is very low priority for now.

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