JP-3753: Refactor extract1d #8961

melanieclarke · 2024-11-14T22:50:50Z

Resolves JP-3753
Resolves JP-3813
Partially addresses JP-3432

Significant refactor for extract1d, to improve code organization, maintainability, and efficiency, as well as set up for PSF-based extraction, to be added in the future.

Tasks

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

codecov · 2024-11-14T23:23:30Z

Codecov Report

Attention: Patch coverage is 95.06579% with 15 lines in your changes missing coverage. Please review.

Project coverage is 66.76%. Comparing base (e38bf84) to head (67bef53).
Report is 65 commits behind head on main.

Files with missing lines	Patch %	Lines
jwst/extract_1d/extract_1d_step.py	88.69%	13 Missing ⚠️
jwst/extract_1d/ifu.py	94.11%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8961      +/-   ##
==========================================
+ Coverage   64.56%   66.76%   +2.20%     
==========================================
  Files         375      375              
  Lines       38890    37948     -942     
==========================================
+ Hits        25109    25336     +227     
+ Misses      13781    12612    -1169

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

melanieclarke · 2024-11-15T15:59:03Z

Initial regression test run here:
https://github.com/spacetelescope/RegressionTests/actions/runs/11846882332
Total test time is about 15 minutes shorter than an equivalent run on main.

There are differences in every x1d and x1dints product, but they are all minor:

DQ array is now updated to set DO_NOT_USE where output flux is NaN. The previous version was not setting DQ in a useful way.
Flux and background errors are different for apertures that include partial pixels. This is because the variance propagation in the current version is incorrect: variances in partial pixels are currently weighted by the pixel fraction, like the science data is, but they should be weighted by pixel fraction squared. The weights are fixed in this version.
In x1dints files, extraction start/stop values are now added to the headers for spectra from all integrations, not just the first integration.
Left and right extraction limit values are now directly set from the aperture edges, where they first go to zero, instead of from the input extraction parameters. This makes a one pixel difference to the MIRI LRS slitless EXTRYSTR/EXTRYSTP values (test_miri_lrs_slitless_tso_spec2[x1dints]).
Approximate extraction start/stop values are added to the headers for spectra extracted with polynomial src_coeff parameters. Before, there was no information about the extraction location in this case (test_nirspec_bots_custom_extraction).

This small change also appears in these results (attempt 1), but probably shouldn't:

Background values are usually zero if there is no background data, but they are now sometimes NaN at the edges of spectra, where they used to be zero.

I will modify the code to make sure all background values are finite. Test attempt 2 at the same link above incorporates this change.

melanieclarke

I am self-reviewing this PR to add some annotations and point out a few changes to specific people. I'm hoping that will make this large PR easier to review. Please let me know if there's anything else I can do to assist or clarify.

jwst/datamodels/container.py

melanieclarke · 2024-12-03T16:05:38Z

docs/jwst/extract_1d/description.rst

+Note that the pipeline takes input start/stop values from the reference files to be
+zero-indexed positions, but all extraction values are recorded in the headers as one-indexed
+values, following FITS header conventions.


Note added to document that FITS header values are 1 off from input and logged start/stop values.

melanieclarke · 2024-12-03T16:08:08Z

docs/jwst/extract_1d/description.rst

+The DQ array is set to DO_NOT_USE for pixels with NaN flux values and zero
+otherwise.


Note added for DQ handling change.

melanieclarke · 2024-12-03T16:09:24Z

docs/jwst/extract_1d/description.rst

+Please note that this is different from the convention used for the cross-dispersion
+start/stop values, which are expected to be inclusive index values. For the example here,
+for horizontal dispersion, `ystart = 325`, `ystop = 335` is equivalent
+to `src_coeff = [[324.5],[335.5]]`.  To include half a pixel more at the top
+and bottom of the aperture, `ystart = 324.5`, `ystop = 335.5` is equivalent
+to `src_coeff = [[324],[336]]`.


Note added to clarify the interpretation difference between start/stop values and polynomial coefficients.

melanieclarke · 2024-12-03T16:10:00Z

docs/jwst/extract_1d/description.rst

-(in microns) or pixel number (pixels in the dispersion direction, with respect to
-the input 2D slit image), which is specified by the parameter ``independent_var``.
-The default is "pixel".  The values of these polynomial functions are pixel numbers in the
-direction perpendicular to dispersion.

 .. _extract-1d-for-ifu:

 Extraction for 3D IFU Data


Documentation changes for IFU extraction in this file are for style only.

melanieclarke · 2024-12-03T18:00:26Z

jwst/extract_1d/ifu.py

Changes in this file are just to remove the image-type reference file, which is no longer supported. Only ASDF reference files are now supported for IFU extractions.

melanieclarke · 2024-12-03T18:02:02Z

jwst/extract_1d/tests/conftest.py

This file adds/relocates some test fixtures for mock data that can pass through various parts of the extraction routines.

melanieclarke · 2024-12-03T18:02:38Z

jwst/extract_1d/tests/test_extract.py

This file adds complete unit test coverage for the jwst.extract_1d.extract module.

melanieclarke · 2024-12-03T18:04:31Z

jwst/extract_1d/tests/test_extract_1d_step.py

This file now includes complete statement coverage for the extract_1d_step interface: all supported modes are tested for basic step completion. NIRISS SOSS takes too long to complete, so it is marked as slow - it will not run with basic CI tests. It will run with the regression tests.

melanieclarke · 2024-12-03T18:06:25Z

jwst/regtest/test_nirspec_bots_extract1d.py

This is a new regression test, added to exercise aperture definition via source coefficients. Truth data on artifactory was generated with the main branch, so any diffs are due to the refactoring in this PR.

github-actions bot added extract_1d NIRSPEC MIRI testing spec2 pipeline regression_testing labels Nov 14, 2024

stscijgbot-jp mentioned this pull request Nov 15, 2024

Refactor/rewrite of extract1d #8802

Closed

melanieclarke mentioned this pull request Nov 15, 2024

JP-3746: Improve background combination for NIRSpec MOS master background #8932

Merged

10 tasks

melanieclarke force-pushed the jp-3753 branch from 24f5f1a to ce895f3 Compare November 18, 2024 20:02

github-actions bot added the datamodels label Nov 19, 2024

melanieclarke force-pushed the jp-3753 branch from 378b149 to 6ea94a3 Compare November 19, 2024 17:52

melanieclarke mentioned this pull request Nov 19, 2024

JP-251: Optimal extraction support for MIRI LRS #8967

Draft

10 tasks

melanieclarke force-pushed the jp-3753 branch 5 times, most recently from c2a3b26 to 421ee4f Compare November 25, 2024 22:32

melanieclarke force-pushed the jp-3753 branch 2 times, most recently from 326fb37 to 20d2c0b Compare November 27, 2024 20:44

github-actions bot added the documentation label Nov 27, 2024

melanieclarke marked this pull request as ready for review November 27, 2024 21:17

melanieclarke requested a review from a team as a code owner November 27, 2024 21:18

melanieclarke force-pushed the jp-3753 branch from bb7b338 to 2eb56dc Compare December 3, 2024 15:27

melanieclarke commented Dec 3, 2024

View reviewed changes

melanieclarke requested review from t-brandt, drlaw1558, jemorrison and hayescr December 3, 2024 18:15

melanieclarke added 23 commits December 12, 2024 16:40

Better npixels estimate for optimal extraction; fix background fit test

e47e853

More unit tests for extract_1d coverage

f71317d

Fix npixels for non-uniform weights

73f3649

More coverage tests

49b9e50

More unit tests for extract

82615f4

Tests for extract_one_slit

74e2e6e

Add change log fragment

b660e81

Make some parameters for create_extraction keywords instead of args

4eb52d6

Test coverage for create_extraction and run_extract1d

ec1c53c

Reformat change note

ee6ebc7

Update docs for extract1d

7a68e62

Add more API docs for extraction tools

a847dde

Make change notes fit on one line

55c5fd3

Minor docs clarifications

83f27fd

Fixes and clarifications from PR review

40e94d7

Refactor extract1d with helper functions for extraction cases

6752d31

Clarify partial pixel weights; check for finite wavelengths

f24f9ea

Fix wavelength check; revise aperture center

61628fe

Don't override explicit user setting for use_source_posn

b6cd760

Add guardrails for smoothing length

5d60520

Add note about use_source_posn for extended sources

2144996

Fix typo

0a3dfad

Reduce repeated code for copying keywords

67bef53

melanieclarke force-pushed the jp-3753 branch from 5ea7aa8 to 67bef53 Compare December 12, 2024 21:40

melanieclarke disabled auto-merge December 12, 2024 21:41

melanieclarke merged commit 363a9ad into spacetelescope:main Dec 12, 2024
23 checks passed

melanieclarke deleted the jp-3753 branch December 12, 2024 22:08

This was referenced Dec 12, 2024

Extract_1d should not override use_source_posn depending on source type #8998

Closed

Extraction aperture centering for NIRSpec BOTS data #7984

Open

Improve extraction from unrectified NIRSpec data #8938

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JP-3753: Refactor extract1d #8961

JP-3753: Refactor extract1d #8961

melanieclarke commented Nov 14, 2024 •

edited

Loading

codecov bot commented Nov 14, 2024 •

edited

Loading

melanieclarke commented Nov 15, 2024 •

edited

Loading

melanieclarke left a comment

melanieclarke Dec 3, 2024

melanieclarke Dec 3, 2024

melanieclarke Dec 3, 2024

melanieclarke Dec 3, 2024 •

edited

Loading

melanieclarke Dec 3, 2024

melanieclarke Dec 3, 2024

melanieclarke Dec 3, 2024

melanieclarke Dec 3, 2024

melanieclarke Dec 3, 2024

		The DQ array is set to DO_NOT_USE for pixels with NaN flux values and zero
		otherwise.

JP-3753: Refactor extract1d #8961

JP-3753: Refactor extract1d #8961

Conversation

melanieclarke commented Nov 14, 2024 • edited Loading

Tasks

stage 1

stage 2

stage 3

other

codecov bot commented Nov 14, 2024 • edited Loading

Codecov Report

melanieclarke commented Nov 15, 2024 • edited Loading

melanieclarke 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

melanieclarke Dec 3, 2024 • 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

melanieclarke commented Nov 14, 2024 •

edited

Loading

codecov bot commented Nov 14, 2024 •

edited

Loading

melanieclarke commented Nov 15, 2024 •

edited

Loading

melanieclarke Dec 3, 2024 •

edited

Loading