-
Notifications
You must be signed in to change notification settings - Fork 32
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
JP-2556: Handle charge spilling out of saturated pixels into neighbors #83
Conversation
Codecov Report
@@ Coverage Diff @@
## main #83 +/- ##
==========================================
+ Coverage 71.79% 71.90% +0.10%
==========================================
Files 16 16
Lines 2308 2317 +9
==========================================
+ Hits 1657 1666 +9
Misses 651 651
Continue to review full report at Codecov.
|
48f7c76
to
05f70de
Compare
not sure why jwst tests are failing, they run when i run them myself. looks like maybe it can't access a url? i opened a pr in romancal to turn off the additional flagging so those tests will pass once that's merged. |
@cshanahan1 The jwst test failures are unrelated. |
@nden yes, ready for review |
07de942
to
f047d1a
Compare
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.
LGTM
The two failures in jwst are not related.
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.
Looks good. Just one question about a potential (but unnecessary) change. Can you tell what kind of effect this has on processing speed, i.e. does it significantly slow down the step at all?
only_sat = np.bitwise_and(gdq_slice, saturated).astype(np.uint8) | ||
box_dim = (n_pix_grow_sat * 2) + 1 | ||
struct = np.ones((box_dim, box_dim)).astype(bool) | ||
dialated = ndimage.binary_dilation(only_sat, structure=struct).astype(only_sat.dtype) |
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.
I like this binary_dilation
method (never seen it before). Is it possible to use it in a mode where you would send it the entire 4D groupdq array after the initial round of flagging is complete (i.e. the loops over groups and integrations is complete) and have it only expand the flags in the 3rd and 4th axes (i.e. image rows and cols)? That way it could be called just once, instead of within the loops here.
Does this need a change to the top-level |
Pixels adjacent to saturated pixels are now also flagged as saturated. By default, every pixel that touches the saturated pixels will also be flagged. The parameter 'n_pix_grow_sat' controls how many pixels out are also flagged, and this can also be set to 0.
Resolves JP-2556