JP-3355 added parameter to extract 1d to override srctype and use point source extraction

[jemorrison]

Resolves JP-3355 and Resolves JP-3340

Closes #7846 and #7815

This PR allows the user to override the srctype and perform POINT source extraction for IFU data.

Checklist for maintainers

• added entry in CHANGES.rst within the relevant release section
• updated or added relevant tests
• updated relevant documentation
• added relevant milestone
• added relevant label(s)
• ran regression tests, post a link to the Jenkins job below.
  How to run regression tests on a PR
  https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/962/
• Make sure the JIRA ticket is resolved properly

[jemorrison]

Will update documentation next

[jemorrison]

I am going to add the work needed for JP-3340 since it is so similar to is needed for this ticket.

[codecov]

Codecov Report

Patch coverage is 11.76% of modified lines.

Files Changed	Coverage
jwst/extract_1d/extract_1d_step.py	0.00%
jwst/extract_1d/ifu.py	20.00%
jwst/extract_1d/extract.py	33.33%

📢 Thoughts on this report? Let us know!.

[tapastro]

Is this worth including over recommending reprocessing spec2 with a srctype step parameter, or having a user modify the header value prior to running extract_1d? I feel like this is an overreach for extract_1d parameters, which is already a bloated list.

[drlaw1558]

This modification is based on a number of user interactions and their frustration with it being so hard to change the spectral extraction behaviour; it's also helpful from a testing perspective.

[jemorrison]

I have had to modify the SRCTYPE to POINT for all the data I have worked with and then set up a specialized extract 1d reference file with a scaled radius. IT IS A PAIN! Having 2 options to extract1d would be so much easier

[jemorrison]

@jmuzerolle @melanieclarke @leonardoubeda I am making these changes to extract1d for MIRI MRS data(option to change to POINT Source, option to scale extraction radius). The NIRSpec aperture correction has multiple radii to use so it could also handle having a scaling radius. But this code change is focused on MIRI because of how the default extraction radii is set. Looking at the NIRSpec App corr files they are still marked as DUMMY and there are 3 fix radius extractions (0.3, 0.5, 1.0). I don't know if the team has done enough study to want to scale the extraction radii. Unless I hear otherwise I will keep these options as only MIRI MRS options

[jemorrison]

This is really an issue with how the data is extracted. If we change it in SRC type step then if there are point source specific steps applied that would happened and we may not want that. I am thinking of NIRSpec pathloss mostly. At least that is my take on it. We just want to have an extraction radius and that is only possible in extract 1d with a POINT source type of data.

[hbushouse]

So given all the commentary regarding how everyone has to rerun extract_1d on their MRS cubes to have the extraction done as a point source, why are we ever setting SRCTYPE='EXTENDED' to begin with? How much would it affect upstream processing if the srctype step were just hardwired to always set it to point source for every MRS exposure? Alternatively, if the extended source version of IFU extraction in extract_1d is totally worthless, then we should consider removing it, or at least changing the logic in the step so that it just always uses point source extraction for MRS, period.

[drlaw1558]

I realize I should clarify; it's not that everyone has to rerun extract_1d. Many programs can use the extended source extraction just fine, or at least it's as good as possible given their scene geometry of fuzzy stuff. It's just that in a number of cases there is a complex extended-type scene with multiple point sources within it, or a single source that turns out to be only slightly extended and can for most purposes be treated as a point source. In those cases it's helpful to be able to use the pipeline to run point-source like extractions at specific locations in the scene rather than users rebuilding the wheel trying to figure out how to use the aperture correction file outside the pipeline.

[jemorrison]

@hbushouse @drlaw1558 I made all the suggested changes. I tested it some. David you might want to test it more.
I will see about getting this run through the regression tests today

[hbushouse]

Still needs a change log entry

[jemorrison]

@leonardoubeda @jmuzerolle @bethanjames
I have made a few changes to extract1d to support MIRI MRS extraction changes.

This pr allows the user for IFU to change from EXTENDED to POINT source (or vice versa). Is this a feature NIRSpec IFU should allow ?

[leonardoubeda]

@jemorrison With regards to the comment:
"This pr allows the user for IFU to change from EXTENDED to POINT source (or vice versa). Is this a feature NIRSpec IFU should allow ?"
I was thinking about the fact that NIRSpec IFU treats EXTENDED and POINT differently when calculating fluxes (#7569) and may be allowing users to select the source type might result in wrong fluxes without intention to do so.

[jemorrison]

@leonardoubeda Yeah I was concerned about that as well. This option to switch to POINT to EXTENDed or vice versa is probably just something MIRI MRS people would want to try. But it is only changing the fact that the extraction can be done using a radius and not the entire FOV of the s3d cube. But since an aperture correction would then also be applied which is
more for POINT like data (unless that is turned off), it might be less confusing to people working with NIRSpec data to just go back and change the SRCTYPE in the calspec2 pipeline. So I will leave this PR as it is and only allowed for MIRI MRS data

[drlaw1558]

LGTM. Tested the pt/extended swap and also that various choices of scaling radius give results that match what I get when overriding the relevant reference file to implement the change by hand. Seems to all be working as expected.

[jemorrison]

@hbushouse Here are the results of the regression tests. The two failures do not seem related to this PR- NIRSpec MSA tests that can not find the msa meta file. https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/958/

[hbushouse]

@jemorrison There are EIGHT failures in your regtest run, not just 2. The first 2 are unrelated, as you say, but the remaining 6 are coming from the new extract_1d code trying to work on WFSS data:

                # --------------------------------------------------------------
                # Data is WFSS
                if input_model[0].meta.exposure.type in extract.WFSS_EXPTYPES:
    
                    # For WFSS level-3, the input is a single entry of a
                    # SourceContainer, which contains a list of multiple
                    # SlitModels for a single source. Send the whole list
                    # into extract1d and put all results in a single product.
                    apcorr_ref = (
                        self.get_reference_file(input_model[0], 'apcorr') if self.apply_apcorr is True else 'N/A'
                    )
    
                    if apcorr_ref == 'N/A':
                        self.log.info('APCORR reference file name is "N/A"')
                        self.log.info('APCORR will NOT be applied')
                    else:
                        self.log.info(f'Using APCORR file {apcorr_ref}')
    
                    extract_ref = 'N/A'
                    self.log.info('No EXTRACT1D reference file will be used')
    
                    result = extract.run_extract1d(
                        input_model,
                        extract_ref,
                        apcorr_ref,
                        self.smoothing_length,
                        self.bkg_fit,
                        self.bkg_order,
                        self.bkg_sigma_clip,
                        self.log_increment,
                        self.subtract_background,
                        self.use_source_posn,
                        self.center_xy,
                        self.ifu_autocen,
                        self.ifu_rfcorr,
                        self.ifu_set_srctype,
>                       self.ifu_rscsale,
                        was_source_model=was_source_model
                    )
E                   AttributeError: 'Extract1dStep' object has no attribute 'ifu_rscsale'

I guess the new ifu_rscale attribute hasn't been added to the step object properly?

[jemorrison]

@hbushouse Ok now there are only 2 failures:
https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/962/

And those failures are because the MSA file can not be found.

[hbushouse]

All looks good now. So I guess the final consensus on whether or not to make the srctype option available for NRS_IFU was no?

[drlaw1558]

I'm getting some major crashes after this merge that I hadn't seen in testing. Extract1d runs in a standalone manner, but not as part of the spec3 pipeline; crashing at
232 if self.ifu_rfcorr:
--> 233 if input_model.meta.exposure.type != "MIR_MRS":
with
AttributeError: No attribute 'exposure'

Was this exposure type test introduced for the first time here?

[hbushouse]

I'm getting some major crashes after this merge that I hadn't seen in testing. Extract1d runs in a standalone manner, but not as part of the spec3 pipeline; crashing at 232 if self.ifu_rfcorr: --> 233 if input_model.meta.exposure.type != "MIR_MRS": with AttributeError: No attribute 'exposure'

Was this exposure type test introduced for the first time here?

It might be due to the fact that the combined s3d file created during level-3 processing doesn't have the exposure attribute carried over into the meta of the data model? It should have all original meta copied over, but that's my guess.

[drlaw1558]

Looks like the issue is that, when called on a single cube, the input is a single Model with all the associated keywords. When called from spec3 the input is a ModelContainer, which presumably needs to access keywords in a different way (and could conceivably change between items in the container?)

[tapastro]

If the input is a ModelContainer, you may need to access a model in the container to check meta properties. Wouldn't this be seen in regression testing?

[hbushouse]

If the input is a ModelContainer, you may need to access a model in the container to check meta properties. Wouldn't this be seen in regression testing?

Those new params are set to None by default, so the if block wouldn't be executed in normal tests. Obviously need a test that exercises the new params.

[jemorrison]

Dang. Ok I will add a test and make figure out what is needed when the s3d is created from multiple channels.

[hbushouse]

But by the time the combined s3d cube gets fed to extract_1d during calwebb_spec3 processing, shouldn't it be just a single model, as opposed to a model container? The original input to calwebb_spec3 will be a model container, loaded from the list of exposures in the ASN file. But I would assume that the output of cube_build is a single model, because the exposures have been combined. Or does it create multiple cubes, one for each channel/band combination? If the input to extract_1d during level-3 is a container, then you just need something like input_model[0].meta.exposure.type, i.e. just pick the first one. They'd better all be the same EXP_TYPE!

[jemorrison]

Working on this now to understand the error. In all cases whether calspec2 or calspec3 the output from cube_build is an IFU model. But it could pass a Container of IFU models if the association contains multiple Channel cubes.

[jemorrison]

How does this seem. I will put in a PR for it if isinstance(input_model, datamodels.IFUImageModel): exp_type = input_model.meta.exposure.type elif isinstance(input_model, ModelContainer): exp_type = input_model[0].meta.exposure.type else: exp_type = None if self.ifu_rfcorr: if exp_type != "MIR_MRS": self.log.warning("The option to apply a residual refringe correction is" f" not supported for {input_model.meta.exposure.type} data.") if self.ifu_rscale is not None: if exp_type != "MIR_MRS": self.log.warning("The option to change the extraction radius is" f" not supported for {input_model.meta.exposure.type} data.") if self.ifu_set_srctype is not None: if exp_type != "MIR_MRS": self.log.warning("The option to change the source type is" f" not supported for {input_model.meta.exposure.type} data.")

…

________________________________ From: Howard Bushouse ***@***.***> Sent: Thursday, September 21, 2023 9:10 AM To: spacetelescope/jwst ***@***.***> Cc: Jane Morrison ***@***.***>; Mention ***@***.***> Subject: [EXT]Re: [spacetelescope/jwst] JP-3355 added parameter to extract 1d to override srctype and use point source extraction (PR #7883) External Email External Email But by the time the combined s3d cube gets fed to extract_1d during calwebb_spec3 processing, shouldn't it be just a single model, as opposed to a model container? The original input to calwebb_spec3 will be a model container, loaded from the list of exposures in the ASN file. But I would assume that the output of cube_build is a single model, because the exposures have been combined. Or does it create multiple cubes, one for each channel/band combination? If the input to extract_1d during level-3 is a container, then you just need something like input_model[0].meta.exposure.type, i.e. just pick the first one. They'd better all be the same EXP_TYPE! — Reply to this email directly, view it on GitHub<#7883 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AENCOWLH3PB2JAKUGDNAPPDX3RRINANCNFSM6AAAAAA4T7AMPE>. You are receiving this because you were mentioned.Message ID: ***@***.***>