JP-3737: Handle micrometeorite flashes #308

drlaw1558 · 2024-10-16T17:13:37Z

This PR addresses JP-3737 by adding an additional check in the jump step to look for micrometeorite flashes, which manifest as bursts of jumps across a large fraction of the detector.

Kept in draft form for now pending potential future modifications.

Tasks

update or add relevant tests
update relevant docstrings and / or docs/ page
Does this PR change any API used downstream? (if not, label with no-changelog-entry-needed)
- write news fragment(s) in changes/: echo "changed something" > changes/<PR#>.<changetype>.rst (see below for change types)
- run regression tests with this branch installed ("git+https://github.com/<fork>/stcal@<branch>")
  - jwst regression test
  - romancal regression test

news fragment change types...

changes/<PR#>.apichange.rst: change to public API
changes/<PR#>.bugfix.rst: fixes an issue
changes/<PR#>.general.rst: infrastructure or miscellaneous change

codecov · 2024-10-16T17:18:28Z

Codecov Report

Attention: Patch coverage is 12.90323% with 27 lines in your changes missing coverage. Please review.

Project coverage is 86.15%. Comparing base (60bd3b8) to head (272dcfc).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/stcal/jump/jump.py	12.90%	27 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #308      +/-   ##
==========================================
- Coverage   86.21%   86.15%   -0.06%     
==========================================
  Files          47       49       +2     
  Lines        8812     8885      +73     
==========================================
+ Hits         7597     7655      +58     
- Misses       1215     1230      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kmacdonald-stsci

There is some code called in two separate places due to a conditional, but can be moved outside the conditionals and called once, since they are called in both the if and else portion of the conditional.

There is also some formatting and style issues.

kmacdonald-stsci · 2024-10-17T12:48:46Z

src/stcal/jump/jump.py

@@ -338,6 +344,12 @@ def detect_jumps(
            )
            log.info("Total showers= %i", num_showers)
            number_extended_events = num_showers
+
+        # This is where we look for micrometeorite flashes if the triggering


Since this is done in both conditionals and done last, it should be moved out side the conditionals completely, instead of invoked twice.

The expand_large_events and find_showers can be moved outside the conditionals for the same reason.

kmacdonald-stsci · 2024-10-17T12:48:53Z

src/stcal/jump/jump.py

+
+        # This is where we look for micrometeorite flashes if the triggering
+        # threshold is less than the entire array
+        if (mmflashfrac < 1):


Since this is done in both conditionals and done last, it should be moved out side the conditionals completely, instead of invoked twice.

The expand_large_events and find_showers can be moved outside the conditionals for the same reason.

kmacdonald-stsci · 2024-10-17T12:50:20Z

src/stcal/jump/jump.py

+# of groups.  Do this by looking for cases where greater than mmflash_pct percent
+# of the array is flagged as 'JUMP_DET' and flag the entire array.
+def find_micrometeorite_flashes(gdq, jump_flag, mmflashfrac):
+    npix = gdq.shape[2]*gdq.shape[3] # Number of pixels in array


Add spaces before and after *.

kmacdonald-stsci · 2024-10-17T12:50:34Z

src/stcal/jump/jump.py

+def find_micrometeorite_flashes(gdq, jump_flag, mmflashfrac):
+    npix = gdq.shape[2]*gdq.shape[3] # Number of pixels in array
+    # Loop over integrations
+    for ii in range(0,gdq.shape[0]):


Add space after ,.

kmacdonald-stsci · 2024-10-17T12:51:29Z

src/stcal/jump/jump.py

+    # Loop over integrations
+    for ii in range(0,gdq.shape[0]):
+        # Loop over groups
+        for jj in range(0,gdq.shape[1]):


Add space after ,.

kmacdonald-stsci · 2024-10-17T12:52:32Z

src/stcal/jump/jump.py

+# Find micrometeorite flashes that light up the entire array in a small number
+# of groups.  Do this by looking for cases where greater than mmflash_pct percent
+# of the array is flagged as 'JUMP_DET' and flag the entire array.
+def find_micrometeorite_flashes(gdq, jump_flag, mmflashfrac):


This is missing a docstring.

Also, start the function with nints, ngroups, nrows, ncols = gdq.shape, then use these more descriptive variable names, rather than shape[k].

Good point- done

kmacdonald-stsci · 2024-10-17T12:52:56Z

src/stcal/jump/jump.py

+    for ii in range(0,gdq.shape[0]):
+        # Loop over groups
+        for jj in range(0,gdq.shape[1]):
+            indx = np.where(gdq[ii,jj,:,:] & jump_flag != 0)


Add spaces after all ,.

kmacdonald-stsci · 2024-10-17T12:53:20Z

src/stcal/jump/jump.py

+            indx = np.where(gdq[ii,jj,:,:] & jump_flag != 0)
+            fraction = float(len(indx[0])) / npix
+            if (fraction > mmflashfrac):
+                gdq[ii,jj,:,:] = gdq[ii,jj,:,:] | jump_flag


Add spaces after all ,.

kmacdonald-stsci · 2024-10-17T12:53:41Z

src/stcal/jump/jump.py

+            fraction = float(len(indx[0])) / npix
+            if (fraction > mmflashfrac):
+                gdq[ii,jj,:,:] = gdq[ii,jj,:,:] | jump_flag
+                log.info("Detected a micrometeorite flash in integ = %i, group= %i", ii, jj)


Why not use an f-string?

If that's the convention I certainly can- done.

drlaw1558 · 2024-10-17T19:51:52Z

Things that remain to be determined:

Check that this function successfully detects more example flashes from the FGS list
Determine whether it needs to be restricted in any way, e.g. to only FULL array or non-TSO data
Determine whether to set mmflashfrac=1.0 by default (i.e., step disabled unless explicitly enabled by param ref file), or to set mmflashfrac=0.1 (for example) to explicitly enable by default unless disabled by param ref file.

Work ongoing to determine these.

drlaw1558 added 2 commits September 30, 2024 17:06

Add flashfrac

22878e6

Merge branch 'main' into jp-3737

495655d

drlaw1558 requested a review from a team as a code owner October 16, 2024 17:13

drlaw1558 marked this pull request as draft October 16, 2024 17:13

github-actions bot added the jump label Oct 16, 2024

stscijgbot-jp mentioned this pull request Oct 16, 2024

Excess Jump Step Flagging in NIRISS data spacetelescope/jwst#8767

Open

kmacdonald-stsci requested changes Oct 17, 2024

View reviewed changes

Code style cleanup

66f0bb6

kmacdonald-stsci approved these changes Oct 17, 2024

View reviewed changes

Safety catch to ensure we dont flag too much with micrometeorite routine

272dcfc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JP-3737: Handle micrometeorite flashes #308

JP-3737: Handle micrometeorite flashes #308

drlaw1558 commented Oct 16, 2024

codecov bot commented Oct 16, 2024 •

edited

Loading

kmacdonald-stsci left a comment •

edited

Loading

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

kmacdonald-stsci Oct 17, 2024

drlaw1558 Oct 17, 2024

drlaw1558 commented Oct 17, 2024

JP-3737: Handle micrometeorite flashes #308

Are you sure you want to change the base?

JP-3737: Handle micrometeorite flashes #308

Conversation

drlaw1558 commented Oct 16, 2024

Tasks

codecov bot commented Oct 16, 2024 • edited Loading

Codecov Report

kmacdonald-stsci left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drlaw1558 commented Oct 17, 2024

codecov bot commented Oct 16, 2024 •

edited

Loading

kmacdonald-stsci left a comment •

edited

Loading