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(flagging): increase default threshold for static weight masking #236

Merged
merged 2 commits into from
Apr 14, 2023

Conversation

ljgray
Copy link
Contributor

@ljgray ljgray commented Apr 3, 2023

Since this is just supposed to catch numerical issues, I'm moving the default thresholds much closer to the max/min float32 values.

I'm also interested in feedback about making this baseline-independent and just spitting out a mask, since it's adding some baseline-dependent flagging at the moment.

@ljgray ljgray requested review from sjforeman and jrs65 April 3, 2023 21:17
@ljgray ljgray marked this pull request as draft April 3, 2023 22:34
@ljgray ljgray force-pushed the ljg/bad-gain-thresh branch 3 times, most recently from 9be90f1 to f68ce9d Compare April 4, 2023 03:43
@ljgray ljgray marked this pull request as ready for review April 4, 2023 18:55
@sjforeman
Copy link
Contributor

The change in the default values seems fine, since the new defaults are used in the rev07 daily config anyway.

I don't have any opinions about making SanitizeWeights baseline-independent, since I don't know what fraction of the data the current SanitizeWeights typically catches, or whether the flagged samples are localized in frequency or time (vs. spread out more randomly, which would incur a larger loss of data by moving to a baseline-independent scheme). I'd be fine with punting that issue to the future...

@ljgray ljgray merged commit 8ecc305 into master Apr 14, 2023
@ljgray ljgray deleted the ljg/bad-gain-thresh branch April 14, 2023 22:16
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