JP-3682: split outlier detection into separate steps for each mode #8722

emolter · 2024-08-22T17:12:58Z

This PR addresses splitting outlier detection into separate steps for coronagraphy, IFU, imaging, spectroscopy, and TSO data. This was motivated by large differences in input parameters and algorithms that are implemented for each mode.

Tasks

codecov · 2024-08-22T17:56:12Z

Codecov Report

Attention: Patch coverage is 67.10526% with 75 lines in your changes missing coverage. Please review.

Project coverage is 76.91%. Comparing base (03ca10f) to head (060ef25).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...st/outlier_detection/outlier_detection_ifu_step.py	16.66%	35 Missing ⚠️
.../outlier_detection/outlier_detection_coron_step.py	76.66%	7 Missing ⚠️
...st/outlier_detection/outlier_detection_tso_step.py	75.00%	7 Missing ⚠️
jwst/pipeline/calwebb_coron3.py	14.28%	6 Missing ⚠️
jwst/pipeline/calwebb_spec3.py	14.28%	6 Missing ⚠️
...t/outlier_detection/outlier_detection_spec_step.py	84.84%	5 Missing ⚠️
...utlier_detection/outlier_detection_imaging_step.py	88.57%	4 Missing ⚠️
jwst/pipeline/calwebb_tso3.py	40.00%	3 Missing ⚠️
jwst/outlier_detection/utils.py	92.59%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8722      +/-   ##
==========================================
- Coverage   76.92%   76.91%   -0.01%     
==========================================
  Files         498      498              
  Lines       45810    45830      +20     
==========================================
+ Hits        35240    35252      +12     
- Misses      10570    10578       +8

Flag	Coverage Δ		*Carryforward flag
nightly	`77.37% <ø> (-0.01%)`	⬇️	Carriedforward from a6e0858

*This pull request uses carry forward flags. Click here to find out more.

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

…-3682

emolter · 2025-01-09T20:26:55Z

regression tests started here https://github.com/spacetelescope/RegressionTests/actions/runs/12697929952

emolter · 2025-01-09T21:31:26Z

@tapastro @melanieclarke let me know what I can do to get the parameter reference files changed over.

braingram

Thanks for taking this on. The code changes look good to me. I left a few comments about suffixes and docs (I didn't finish looking over all the docs changes). The refactoring looks much cleaner.

For the reference files perhaps @stscieisenhamer has some suggestions on how to handle the transition from a single pars-outlierdetectionstep reftype to the multiple pars-outlierdetectionifustep etc pars files. To propose a plan maybe we define the new reftypes on crds, start using them with this PR and leave the existing pars-outlierdetectionstep reftype up (so previous cal versions can still use the context) for some period of time (it looks like the unused drizpars is still in the current context).
Related to the transition, I think this refactoring warrants a migration guide in the docs. Something that describes to a user that if they previously used strun outlierdetection they now need to do .... Is that in the updated docs (and I missed it)?

braingram · 2025-01-10T14:40:26Z

jwst/pipeline/calwebb_spec3.py

@@ -85,8 +86,10 @@ def process(self, input):
        # Setup sub-step defaults
        self.master_background.suffix = 'mbsub'
        self.mrs_imatch.suffix = 'mrs_imatch'
-        self.outlier_detection.suffix = 'crf'
-        self.outlier_detection.save_results = self.save_results
+        self.outlier_detection_spec.suffix = 'crf'


Is the suffix now one-to-one for the split steps (eg outlier_detection_spec always uses crf)? If so I suggest we define them in the class instead of assigning in the pipeline.

yes it's now 1:1, will define them in the spec of each step

braingram · 2025-01-10T14:42:50Z

jwst/outlier_detection/utils.py

+    """Minimal base class holding common methods for outlier detection steps."""
+
+    @abstractmethod
+    def search_attr(self, attr, **kwargs):


What required defining this method?

good catch. These were carried over from the draft of this PR several months ago. They no longer appear to be necessary, if they ever were necessary in the first place.

braingram · 2025-01-10T14:42:59Z

jwst/outlier_detection/utils.py

+        pass
+
+    @abstractmethod
+    def _make_output_path(self):


What required defining this method?

same as above

docs/jwst/outlier_detection_tso/arguments_tso.rst

braingram · 2025-01-10T14:48:10Z

jwst/outlier_detection/outlier_detection_tso_step.py

+class OutlierDetectionTSOStep(Step, OutlierDetectionStepBase):
+    """Flag outlier bad pixels and cosmic rays in DQ array of each input image.
+    Input images can be listed in an input association file or already opened
+    with a ModelContainer.  DQ arrays are modified in place.


Is ModelContainer supported?

no, I will update the docstring

docs/jwst/outlier_detection_coron/arguments_coron.rst

jwst/lib/suffix.py

emolter · 2025-01-10T16:28:34Z

I'm working through regtest failures now, most of which are due to the need to change e.g. --steps.outlier_detection.save_results to --steps.outlier_detection_spec.save_results

braingram · 2025-01-10T19:17:55Z

docs/jwst/outlier_detection/outlier_detection.rst

+   step = OutlierDetectionCoronStep()
+   step.call(input.fits, save_intermediate_results=True)


Suggested change

step = OutlierDetectionCoronStep()

step.call(input.fits, save_intermediate_results=True)

OutlierDetectionCoronStep.call(input.fits, save_intermediate_results=True)

call is a class method.

yep, my bad, will fix

braingram · 2025-01-10T19:19:21Z

docs/jwst/outlier_detection/outlier_detection.rst

+   from jwst.pipeline import calwebb_image3
+   calwebb_image3(input.fits, steps={'outlier_detection_coron': {'save_intermediate_results': True}})


Suggested change

from jwst.pipeline import calwebb_image3

calwebb_image3(input.fits, steps={'outlier_detection_coron': {'save_intermediate_results': True}})

from jwst.pipeline import Image3Pipeline

Image3Pipeline.call(input.fits, steps={'outlier_detection_coron': {'save_intermediate_results': True}})

…-3682

kmacdonald-stsci · 2025-01-13T13:56:32Z

jwst/outlier_detection/outlier_detection_imaging_step.py

+
+    Parameters
+    -----------
+    input_data : asn file or ~jwst.datamodels.ModelContainer


Should this be input_models?

kmacdonald-stsci · 2025-01-13T13:59:44Z

jwst/outlier_detection/outlier_detection_spec_step.py

+    with a ModelContainer.  DQ arrays are modified in place.
+    Parameters
+    -----------
+    input_data : ~jwst.datamodels.ModelContainer or str


Should this be input_models?

kmacdonald-stsci · 2025-01-13T14:00:32Z

jwst/outlier_detection/outlier_detection_tso_step.py

-    See `OutlierDetectionStep.spec` for documentation of these arguments.
+    Parameters
+    -----------
+    input_data : ~jwst.datamodels.CubeModel or str


Should this be input_model?

kmacdonald-stsci · 2025-01-13T14:03:21Z

jwst/outlier_detection/utils.py

+    """Minimal base class holding common methods for outlier detection steps."""
+
+    def _get_asn_id(self, input_models):
+        """Find association ID for any allowed input model type,


Docstring missing "Parameter" section describing the input parameter.

kmacdonald-stsci · 2025-01-13T14:03:34Z

jwst/outlier_detection/utils.py

+        return
+
+    def _set_status(self, input_models, status):
+        """Record step exit status, handling both filenames and open models."""


Docstring missing "Parameters" section describing the input parameters.

…-3682

github-actions bot added outlier_detection pipeline stpipe testing image3 pipeline coron3 pipeline spec3 pipeline tso3 pipeline labels Aug 22, 2024

github-actions bot added the documentation label Aug 22, 2024

emolter added this to the Build 11.2 milestone Aug 22, 2024

stscijgbot-jp mentioned this pull request Aug 22, 2024

Split OutlierDetectionStep into multiple steps #8634

Open

melanieclarke removed this from the Build 11.2 milestone Dec 12, 2024

JP-3682: split outlier detection into multiple steps

319e349

emolter force-pushed the JP-3682 branch from 32b8c51 to 319e349 Compare January 9, 2025 19:01

emolter added this to the Build 11.3 milestone Jan 9, 2025

emolter added 3 commits January 9, 2025 15:22

restructured docs

26079af

Merge branch 'main' of https://github.com/spacetelescope/jwst into JP…

8d58c8b

…-3682

added changelog entry

8f8cf6f

emolter requested a review from braingram January 9, 2025 20:27

split towncrier fragments by new steps

ec761aa

github-actions bot added installation automation Continuous Integration (CI) and testing automation tools labels Jan 9, 2025

emolter added 2 commits January 9, 2025 15:44

update current changes to use new towncrier types

85f6ae4

fix suffixes

d547bb3

emolter marked this pull request as ready for review January 9, 2025 21:30

emolter requested review from a team as code owners January 9, 2025 21:30

emolter requested a review from melanieclarke January 9, 2025 21:30

emolter requested review from tapastro and jemorrison January 9, 2025 21:30

braingram reviewed Jan 10, 2025

View reviewed changes

emolter added 2 commits January 10, 2025 12:19

fixes per Brett review

327db0e

attempted fix for failing regtests

a6e0858

github-actions bot added NIRSPEC MIRI regression_testing labels Jan 10, 2025

added transition guide, more attempted regtest fixes

48f6ac2

braingram reviewed Jan 10, 2025

View reviewed changes

emolter added 2 commits January 10, 2025 14:36

fixed syntax for call in transition guide

9edcceb

Merge branch 'main' of https://github.com/spacetelescope/jwst into JP…

3c1e329

…-3682

kmacdonald-stsci approved these changes Jan 13, 2025

View reviewed changes

emolter added 2 commits January 13, 2025 10:42

Merge branch 'main' of https://github.com/spacetelescope/jwst into JP…

624c7bf

…-3682

fixes from Ken review

060ef25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JP-3682: split outlier detection into separate steps for each mode #8722

JP-3682: split outlier detection into separate steps for each mode #8722

emolter commented Aug 22, 2024 •

edited

Loading

codecov bot commented Aug 22, 2024 •

edited

Loading

emolter commented Jan 9, 2025

emolter commented Jan 9, 2025

braingram left a comment

braingram Jan 10, 2025

emolter Jan 10, 2025

braingram Jan 10, 2025

emolter Jan 10, 2025

braingram Jan 10, 2025

emolter Jan 10, 2025

braingram Jan 10, 2025

emolter Jan 10, 2025

emolter commented Jan 10, 2025

braingram Jan 10, 2025

emolter Jan 10, 2025

braingram Jan 10, 2025

kmacdonald-stsci Jan 13, 2025

kmacdonald-stsci Jan 13, 2025

kmacdonald-stsci Jan 13, 2025

kmacdonald-stsci Jan 13, 2025

kmacdonald-stsci Jan 13, 2025

		step = OutlierDetectionCoronStep()
		step.call(input.fits, save_intermediate_results=True)

	step = OutlierDetectionCoronStep()
	step.call(input.fits, save_intermediate_results=True)
	OutlierDetectionCoronStep.call(input.fits, save_intermediate_results=True)

		from jwst.pipeline import calwebb_image3
		calwebb_image3(input.fits, steps={'outlier_detection_coron': {'save_intermediate_results': True}})

JP-3682: split outlier detection into separate steps for each mode #8722

Are you sure you want to change the base?

JP-3682: split outlier detection into separate steps for each mode #8722

Conversation

emolter commented Aug 22, 2024 • edited Loading

Tasks

codecov bot commented Aug 22, 2024 • edited Loading

Codecov Report

emolter commented Jan 9, 2025

emolter commented Jan 9, 2025

braingram left a 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

emolter commented Jan 10, 2025

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

emolter commented Aug 22, 2024 •

edited

Loading

codecov bot commented Aug 22, 2024 •

edited

Loading