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

Minor fix in sky subtraction when fiber has ivar=0 #920

Merged
merged 1 commit into from Mar 16, 2020
Merged

Conversation

julienguy
Copy link
Contributor

Use copy of frame instead of applying twice the sky subtraction on the same frame with a correction coefficient; this was a source of a bug when all fiber has ivar=0.

…e same frame with a correction coefficient; this was a source of a bug when all fiber has ivar=0
@sbailey
Copy link
Contributor

sbailey commented Mar 16, 2020

tested on previously bad cframes, looks good.

@sbailey sbailey merged commit 9e826a6 into master Mar 16, 2020
@sbailey sbailey deleted the skysubfix branch March 16, 2020 23:45
@@ -1160,7 +1159,6 @@ def subtract_sky(frame, skymodel, throughput_correction = False, default_through

if mcoeferr>np.abs(mcoef-1) :
log.warning("throughput corr error = %5.4f is too large compared to the correction value = %5.4f for fiber #%03d, do not apply correction"%(mcoeferr,(mcoef-1),fiber))
throughput_correction_value = default_throughput_correction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

turns out this was needed, because otherwise throughput_correction_value isn't set in this case and the code crashes a few lines later when it is used. I updated directly to master to set this to 1.0. We can chase separately what causes this warning special case and whether we should do something different about it, but for the moment I was looking for the minimal fix to get back and running.

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.

None yet

2 participants