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

Ql countqas #600

Merged
merged 9 commits into from Apr 30, 2018
Merged

Ql countqas #600

merged 9 commits into from Apr 30, 2018

Conversation

gdhungana
Copy link
Contributor

This PR addresses the issues #580 and #582 . The new format for the metrics block on those QAs are

  • Count Pix: {'LITFRAC_AMP': [0.35, 0.37, 0.33, 0.35],
    'LITFRAC_STATUS': 'NORMAL',
    'NPIX_AMP': [1468029, 1555863, 1410426, 1488293]}

  • Count Bins: {'GOOD_FIBER': [1, 1, 1....,1, 1], 'NGOODFIB': 500,
    'NGOODFIB_STATUS': 'NORMAL'}

  • This also updates the respective tests and static plots to address new (and single) thresholds and relevant keys.

@rkehoe @Srheft Please review when you get a chance. Let me know if this meets the requirements of those issues.

@Srheft
Copy link
Contributor

Srheft commented Apr 25, 2018

LITFRAC_AMP is the fraction of each amp that is illuminated.
NPIX_AMP: number of pixels per amp (>5sigma) seems to not have much use as the illuminated fraction does the critical check. I'd vote for removing them unless another more informative quantity could be derived from them.
Note that the configuration files on github are updated to accommodate the new check HDU in the branch called "ql_HDUsnMore".

@rkehoe
Copy link
Contributor

rkehoe commented Apr 26, 2018

@Srheft Thanks for the comment. I think NPIX_AMP is fairly superfluous and harder to interpret than the new scalar metric LITFRAC_AMP, and agree it should be removed. I will make this modification once I am done with my review of the code and testing.

@rkehoe
Copy link
Contributor

rkehoe commented Apr 27, 2018

I removed NPIX_AMP array from the output metrics and updated tests/plots. This is ready to merge pending travis test.

@rkehoe
Copy link
Contributor

rkehoe commented Apr 30, 2018

Merging.

@rkehoe rkehoe merged commit b4de25a into master Apr 30, 2018
@rkehoe rkehoe deleted the ql_countqas branch May 9, 2018 15:35
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

3 participants