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

fix template fibermap mag; fix quickgen outputs #151

Merged
merged 5 commits into from
Aug 24, 2016
Merged

fix template fibermap mag; fix quickgen outputs #151

merged 5 commits into from
Aug 24, 2016

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Aug 24, 2016

This PR fixes the MAG calculation for the simulated fibermap, which was broken as part of PR #132. The previous flattening/slicing/stacking logic was wrong and the same magnitudes were replicated across multiple targets. This was the core problem for the integration test failures.

Other changes that came long for the ride as a result of debugging this:

  • adds exposure flavor "std" to newexp-desi to simulate a bunch of standards; useful for debugging
  • several quickgen fixes:
    • include fibermap in frame, cframe output
    • include CAMERA header keyword
    • fiberflats are referenced to 1.0 not 0.0

@moustakas moustakas merged commit 498bd40 into master Aug 24, 2016
@moustakas
Copy link
Member

The changes to the code look good. I removed some now-obsolete commented out code and also made the default redshift to be zero in desisim.io.empty_metatable, to avoid some harmless but annoying divide-by-zero errors downstream.

My only concern is that test_obs.test_newexp seems to occasionally fail for reasons that I don't understand (and for reasons that seem to be unreproducible). We might think about changing/updating this test.

@moustakas moustakas deleted the fixdaily branch August 24, 2016 11:33
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