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-3347: Improve spectral outlier detection #8828

Merged
merged 12 commits into from
Oct 17, 2024

Conversation

melanieclarke
Copy link
Collaborator

@melanieclarke melanieclarke commented Sep 24, 2024

Resolves JP-3347

Modify outlier detection for spectral data, in order to threshold outliers for an exposure against an error value computed across exposures, instead of the input error for the exposure itself. This is intended to help address extreme outliers that have both very high data values and very high error values in the input exposure.

The process for resampled data is:

  • drizzle the error array the same way as the science, to approximate a resampled error for each exposure
  • median combine the drizzled error arrays across exposures, in the same way the drizzled science arrays are median combined
  • blot the median error back to the input data coordinates, in the same way the median science image is handled
  • compare (input data - blotted median) to blotted median error for thresholding purposes.

For outlier detection on spectra without resampling (not commonly used), the median error is directly computed across the input exposures and used for thresholding.

Tasks

  • request a review from someone specific, to avoid making the maintainers review every PR
  • add a build milestone, i.e. Build 11.3 (use the latest build if not sure)
  • Does this PR change user-facing code / API? (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)
    • update or add relevant tests
    • update relevant docstrings and / or docs/ page
    • start a regression test and include a link to the running job (click here for instructions)
      • Do truth files need to be updated ("okified")?
        • after the reviewer has approved these changes, run okify_regtests to update the truth files
  • if a JIRA ticket exists, make sure it is resolved properly
news fragment change types...
  • changes/<PR#>.general.rst: infrastructure or miscellaneous change
  • changes/<PR#>.docs.rst
  • changes/<PR#>.stpipe.rst
  • changes/<PR#>.datamodels.rst
  • changes/<PR#>.scripts.rst
  • changes/<PR#>.fits_generator.rst
  • changes/<PR#>.set_telescope_pointing.rst
  • changes/<PR#>.pipeline.rst

stage 1

  • changes/<PR#>.group_scale.rst
  • changes/<PR#>.dq_init.rst
  • changes/<PR#>.emicorr.rst
  • changes/<PR#>.saturation.rst
  • changes/<PR#>.ipc.rst
  • changes/<PR#>.firstframe.rst
  • changes/<PR#>.lastframe.rst
  • changes/<PR#>.reset.rst
  • changes/<PR#>.superbias.rst
  • changes/<PR#>.refpix.rst
  • changes/<PR#>.linearity.rst
  • changes/<PR#>.rscd.rst
  • changes/<PR#>.persistence.rst
  • changes/<PR#>.dark_current.rst
  • changes/<PR#>.charge_migration.rst
  • changes/<PR#>.jump.rst
  • changes/<PR#>.clean_flicker_noise.rst
  • changes/<PR#>.ramp_fitting.rst
  • changes/<PR#>.gain_scale.rst

stage 2

  • changes/<PR#>.assign_wcs.rst
  • changes/<PR#>.badpix_selfcal.rst
  • changes/<PR#>.msaflagopen.rst
  • changes/<PR#>.nsclean.rst
  • changes/<PR#>.imprint.rst
  • changes/<PR#>.background.rst
  • changes/<PR#>.extract_2d.rst
  • changes/<PR#>.master_background.rst
  • changes/<PR#>.wavecorr.rst
  • changes/<PR#>.srctype.rst
  • changes/<PR#>.straylight.rst
  • changes/<PR#>.wfss_contam.rst
  • changes/<PR#>.flatfield.rst
  • changes/<PR#>.fringe.rst
  • changes/<PR#>.pathloss.rst
  • changes/<PR#>.barshadow.rst
  • changes/<PR#>.photom.rst
  • changes/<PR#>.pixel_replace.rst
  • changes/<PR#>.resample_spec.rst
  • changes/<PR#>.residual_fringe.rst
  • changes/<PR#>.cube_build.rst
  • changes/<PR#>.extract_1d.rst
  • changes/<PR#>.resample.rst

stage 3

  • changes/<PR#>.assign_mtwcs.rst
  • changes/<PR#>.mrs_imatch.rst
  • changes/<PR#>.tweakreg.rst
  • changes/<PR#>.skymatch.rst
  • changes/<PR#>.exp_to_source.rst
  • changes/<PR#>.outlier_detection.rst
  • changes/<PR#>.tso_photometry.rst
  • changes/<PR#>.stack_refs.rst
  • changes/<PR#>.align_refs.rst
  • changes/<PR#>.klip.rst
  • changes/<PR#>.spectral_leak.rst
  • changes/<PR#>.source_catalog.rst
  • changes/<PR#>.combine_1d.rst
  • changes/<PR#>.ami.rst

other

  • changes/<PR#>.wfs_combine.rst
  • changes/<PR#>.white_light.rst
  • changes/<PR#>.cube_skymatch.rst
  • changes/<PR#>.engdb_tools.rst
  • changes/<PR#>.guider_cds.rst

Copy link

codecov bot commented Sep 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.84%. Comparing base (32f744d) to head (8d3dc6d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8828      +/-   ##
==========================================
+ Coverage   61.79%   61.84%   +0.04%     
==========================================
  Files         377      377              
  Lines       38827    38876      +49     
==========================================
+ Hits        23992    24041      +49     
  Misses      14835    14835              

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

@melanieclarke melanieclarke changed the title WIP - JP-3347: Improve spectral outlier detection JP-3347: Improve spectral outlier detection Oct 2, 2024
@melanieclarke
Copy link
Collaborator Author

melanieclarke commented Oct 2, 2024

@melanieclarke
Copy link
Collaborator Author

Regression tests show minor differences for slit spectra in spec3 tests (NIRSpec FS and MOS, MIRI LRS), downstream of outlier detection, as expected. The MIRI differences are not significant - there are a few pixels slightly different, near a saturated region of the spectrum. For the NIRSpec data, some new outliers are appropriately flagged and good signal does not appear to be newly flagged, with one exception.

The spectrum in this test:
jwst.regtest.test_nirspec_fs_spec3_moving_target.[stable-deps] test_nirspec_fs_spec3_moving_target[x1d]
shows some new over-aggressive flagging at the core of the trace. This is similar to issues described in JP-3347, and can be fixed with the recommended parameter override scale="3.0 2.0". The NIRSpec team plans to follow up this PR with new default parameters, so I think this is not a concern for these changes.

@melanieclarke
Copy link
Collaborator Author

I will leave this PR in draft for now, because I'd like to hear back from the MIRI team to make sure they are okay with these changes, but I think this is otherwise ready for review.

Copy link
Collaborator

@emolter emolter left a comment

Choose a reason for hiding this comment

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

I couldn't find anything wrong with the code, looks excellent to me. But I suppose the behavior almost exactly mimics what is done with the data. Most of my comments are minor and docstring-related. I'll leave it to the INS teams to decide if the regtest outputs look good or not

docs/jwst/outlier_detection/outlier_detection_spec.rst Outdated Show resolved Hide resolved
docs/jwst/outlier_detection/outlier_detection_spec.rst Outdated Show resolved Hide resolved
docs/jwst/outlier_detection/outlier_detection_spec.rst Outdated Show resolved Hide resolved
docs/jwst/outlier_detection/outlier_detection_spec.rst Outdated Show resolved Hide resolved
jwst/outlier_detection/utils.py Outdated Show resolved Hide resolved
jwst/outlier_detection/utils.py Outdated Show resolved Hide resolved
jwst/outlier_detection/utils.py Outdated Show resolved Hide resolved
jwst/outlier_detection/utils.py Show resolved Hide resolved
jwst/resample/resample.py Outdated Show resolved Hide resolved
@melanieclarke
Copy link
Collaborator Author

melanieclarke commented Oct 15, 2024

MIRI is okay with these changes, so this will be the new standard behavior for all slit spectra (no additional optional parameter needed).

Rerunning regression tests with new NIRSpec outlier detection defaults here:
https://github.com/spacetelescope/RegressionTests/actions/runs/11350735089

With the updated parameters, the overaggressive flagging in test_nirspec_fs_spec3_moving_target[x1d] is no longer present. The other changes still look as expected for this branch.

@melanieclarke melanieclarke marked this pull request as ready for review October 15, 2024 18:53
@melanieclarke melanieclarke requested a review from a team as a code owner October 15, 2024 18:53
@melanieclarke
Copy link
Collaborator Author

@emolter - did you have any further comments on this PR?

Copy link
Collaborator

@emolter emolter left a comment

Choose a reason for hiding this comment

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

Looks good to me

jwst/outlier_detection/utils.py Outdated Show resolved Hide resolved
@melanieclarke
Copy link
Collaborator Author

@tapastro - do you want to review for the maintainers?

@tapastro tapastro merged commit d4c7f6b into spacetelescope:main Oct 17, 2024
27 checks passed
@melanieclarke melanieclarke deleted the jp-3347 branch October 17, 2024 18:11
hayescr pushed a commit to hayescr/jwst that referenced this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants