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

JP-3737: Handle micrometeorite flashes #308

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/stcal/jump/jump.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
min_diffs_single_pass=10,
mask_persist_grps_next_int=True,
persist_grps_flagged=25,
mmflashfrac=1.0,
):
"""
This is the high-level controlling routine for the jump detection process.
Expand Down Expand Up @@ -221,6 +222,11 @@
min_diffs_single_pass : int
The minimum number of groups to switch to flagging all outliers in a single pass.

mmflashfrac: float
Fraction of the array in a given group that has to have a jump detected
in order to flag the entire array as a jump. This is designed to deal with
sporadic micrometeorite flashes.

Returns
-------
gdq : int, 4D array
Expand Down Expand Up @@ -338,6 +344,12 @@
)
log.info("Total showers= %i", num_showers)
number_extended_events = num_showers

# This is where we look for micrometeorite flashes if the triggering
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

# threshold is less than the entire array
if (mmflashfrac < 1):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

gdq = find_micrometeorite_flashes(gdq, jump_flag, mmflashfrac)

Check warning on line 351 in src/stcal/jump/jump.py

View check run for this annotation

Codecov / codecov/patch

src/stcal/jump/jump.py#L351

Added line #L351 was not covered by tests

else:
yinc = int(n_rows // n_slices)
slices = []
Expand Down Expand Up @@ -503,6 +515,12 @@
)
log.info("Total showers= %i", num_showers)
number_extended_events = num_showers

# This is where we look for micrometeorite flashes if the triggering
# threshold is less than the entire array
if (mmflashfrac < 1):
gdq = find_micrometeorite_flashes(gdq, jump_flag, mmflashfrac)

Check warning on line 522 in src/stcal/jump/jump.py

View check run for this annotation

Codecov / codecov/patch

src/stcal/jump/jump.py#L522

Added line #L522 was not covered by tests

elapsed = time.time() - start
log.info("Total elapsed time = %g sec", elapsed)

Expand All @@ -514,6 +532,21 @@
# Return the updated data quality arrays
return gdq, pdq, total_primary_crs, number_extended_events, stddev

# 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):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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].

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point- done

npix = gdq.shape[2]*gdq.shape[3] # Number of pixels in array

Check warning on line 539 in src/stcal/jump/jump.py

View check run for this annotation

Codecov / codecov/patch

src/stcal/jump/jump.py#L539

Added line #L539 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add spaces before and after *.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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

Check warning on line 541 in src/stcal/jump/jump.py

View check run for this annotation

Codecov / codecov/patch

src/stcal/jump/jump.py#L541

Added line #L541 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add space after ,.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

# Loop over groups
for jj in range(0,gdq.shape[1]):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add space after ,.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

indx = np.where(gdq[ii,jj,:,:] & jump_flag != 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add spaces after all ,.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

fraction = float(len(indx[0])) / npix
if (fraction > mmflashfrac):
gdq[ii,jj,:,:] = gdq[ii,jj,:,:] | jump_flag
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add spaces after all ,.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

log.info("Detected a micrometeorite flash in integ = %i, group= %i", ii, jj)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use an f-string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

return gdq

Check warning on line 549 in src/stcal/jump/jump.py

View check run for this annotation

Codecov / codecov/patch

src/stcal/jump/jump.py#L543-L549

Added lines #L543 - L549 were not covered by tests

def flag_large_events(
gdq,
Expand Down
Loading