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

stdstar correct sky and flat fiber; scale ivar #1817

Merged
merged 6 commits into from Aug 5, 2022
Merged

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Aug 3, 2022

This PR fixes #1810 for standard star fitting:

  • use the correct fiber when applying the fiberflat and sky subtraction
  • when scaling the flux by the fiberflat, also scale the ivar to match

Testing on all exposures of 20210531, this results in a distinct improvement in the scatter of the data-model G-R colors:
image

The test outputs are in /global/cfs/cdirs/desi/users/sjbailey/spectro/redux/stdstar, which used guadalupe inputs and re-ran just the stdstar fitting step for each tile.

@sbailey sbailey requested a review from julienguy August 3, 2022 22:07
@sbailey sbailey added this to In progress in Himalayas via automation Aug 3, 2022
Copy link
Contributor

@julienguy julienguy left a comment

Choose a reason for hiding this comment

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

Thank you for the bug fix. It does indeed improve the fit of standard stars.
I see the original code and this new version are missing the transformation to convert fiberflat wavelength from the Kitt Peak frame to the solar system barycenter. It is included in the routine fiberflat.apply_fiberflat(frame, fiberflat). It would be better to use directly this function here.

@sbailey
Copy link
Contributor Author

sbailey commented Aug 4, 2022

I updated to use apply_fiberflat and subtract_sky, which led to other changes for SkyModel object slicing and updating the tests and fixing a bug in fiberbitmasking when working on a sliced subset of a full frame, but in the end the improvements are similar:
image

@sbailey
Copy link
Contributor Author

sbailey commented Aug 5, 2022

For the record: the reason why the results are different when using apply_fiberflat and subtract_sky is that those also propagate the error model from the fiberflat and sky models, which was previously ignored when doing flux/fiberflat - sky and ivar*fiberflat**2.

Copy link
Contributor

@julienguy julienguy left a comment

Choose a reason for hiding this comment

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

Great. I checked the code and successfully ran std star fit for one tile and got slightly reduced rms.

@julienguy julienguy merged commit 3de0f52 into master Aug 5, 2022
Himalayas automation moved this from In progress to Done Aug 5, 2022
@sbailey sbailey deleted the stdflatfiber branch November 9, 2022 02:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

stdstars using wrong fiberflat and sky
2 participants