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

Simulation realism: part 1 #46

Merged
merged 31 commits into from
Aug 28, 2016
Merged

Simulation realism: part 1 #46

merged 31 commits into from
Aug 28, 2016

Conversation

dkirkby
Copy link
Member

@dkirkby dkirkby commented Aug 27, 2016

This is the first major piece of adding simulation realism. The simulation now knows about the plate scale variation across the focal plane and can locate simulated sources on the focal plane, resulting in the correct variation of on-sky fiber area and shape.

The next step is to upgrade the fiber acceptance fraction calculations #5 so that the source throughput reflects focal plane position (currently only the sky background level reflects the on-sky fiber area).

Fixes #24.

dkirkby added 28 commits August 24, 2016 08:16
@dkirkby
Copy link
Member Author

dkirkby commented Aug 27, 2016

@sbailey @moustakas I am planning to merge this tomorrow so I can start work on part 2, but let me know if I should hold off to allow time for review. Integration with desisim will not start until the dust settles on part 2.

@moustakas
Copy link
Member

I'm not set up right now to test the code, but I did read through all the proposed changes. Everything looks good as far as I can tell. I wonder whether users will get tripped-up by having to specify the units with the exposure time (lines 30-31 of driver.py); for example, do I specify "minutes" with "m" or "min"? In any case I'm looking forward to the hook-up with desisim.

@dkirkby
Copy link
Member Author

dkirkby commented Aug 28, 2016

I actually made the same mistake ('m' vs 'min') when I was testing, so I just improved the error handling:

% quickspecsim -c desi --exposure-time=15m
Quantity "15m" is not convertible to s.

@dkirkby dkirkby merged commit c69a1e5 into master Aug 28, 2016
@dkirkby dkirkby deleted the platescale_variation branch August 28, 2016 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants