search

desihub / specsim

Quick simulations of spectrograph response
2 stars 9 forks source link

Add support for multi-fiber simulations #57

Closed dkirkby closed 7 years ago

dkirkby commented 7 years ago

Work in progress, building on the new support for parallel computation of fiberloss fractions.

dkirkby commented 7 years ago

I think this is ready to review now @sbailey. At this point, I am just trying to get the API right before overhauling quickgen, but I expect some performance tuning will still be needed under the hood. The best starting point is the new examples notebook in docs/nb/SimulationExamples.ipynb.

dkirkby commented 7 years ago

I built a preview of the updated docs at http://specsim.readthedocs.io/en/multifiber/

sbailey commented 7 years ago

Looks good. The new docs/nb/SimulationExamples.ipynb is especially useful for new users to get started. I suggest adding a link to that from the Users Guide section of the documentation generated for readthedocs.

One minor suggestion for the notebook: I found it somewhat odd to go through how to change the airmass and exposure time without knowing what input source was being simulated. Around cell 5 where you first run desi.simulate() it would be nice to know what was being simulated (constant AB mag source?). Fine to merge with or without that.

dkirkby commented 7 years ago

Thanks for the suggestions, which I implemented. Once the travis tests have passed, I will merge and then tag & release v0.8.