-
Notifications
You must be signed in to change notification settings - Fork 7
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
DM-49497: Add check for valid BFK #303
Conversation
python/lsst/cp/pipe/cpBfk.py
Outdated
# Edges should converge to zero | ||
valid = np.all(np.isclose(kernelEdges, 0, rtol=5*kernelEdgeStd)) | ||
# Kernel should be negative | ||
valid = np.all(bfk.ampKernels[ampName] <= 5*kernelEdgeStd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a *=
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! Thanks :)
python/lsst/cp/pipe/cpBfk.py
Outdated
xv, yv = np.meshgrid(range(kernelSize), range(kernelSize)) | ||
# Get a mask for a 3px picture frame | ||
valid = True | ||
if kernelSize >= 7: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should any of this be configurable (the 3x3, the 7 size, the 5 times std) or you think these are fixed-fixed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 5 sigma can be a config, but the 3px shouldn't be. The kernel should be just big enough to have 3 px on the edge that go to zero. 7 is then just the minimum size (3*2 + 1)
0a3f7ac
to
3669bfb
Compare
67e8d63
to
b12f6de
Compare
b12f6de
to
ccca598
Compare
ccca598
to
fb3776c
Compare
No description provided.