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

Revise MRS 1d residual fringe correction to run automatically and populate x1d extension #8920

Open
stscijgbot-jp opened this issue Oct 24, 2024 · 12 comments

Comments

@stscijgbot-jp
Copy link
Collaborator

Issue JP-3790 was created on JIRA by David Law:

After considerable discussion within the MIRI team, we would like to revise the current implementation of the 1d residual fringe correction within the extract_1d step.

At present, ifu_rfcorr is an optional argument that users can provide to apply the 1d correction.  This correction is off by default as it can remove periodic features in some cases with molecular bands, but this also means that the vast majority of users need to rerun both cube building (to create per-band cubes) and spectral extraction in order to get clean spectra.

The requested change is to have 1d residual fringe correction always run by default, but to provide two versions of the extracted spectra.  The first copy, in the usual EXTRACT1D 1st extension, will be the normal spectrum with no defringing applied.  The defringed spectra (science, background, etc) will be in a new 2nd extension with some sensible FITS EXTNAME.  Maybe RFCORR1D?

This way, users can simply choose for themselves which to use without rerunning the pipeline, or requiring additional MAST data products.

A few details:

  • It may be worth retaining the ifu_rfcorr option flag in extract_1d for a little while, but when set to TRUE simply log a statement informing users of the change in behaviour.  This at least won't break previous code.
  • At the same time, the MIRI team will deliver a parameter reference file update that changes the default Spec3 cubes to use per-band cubes for which residual fringe correction works well.  This will have a significant impact on DMS and MAST.
  • We'll presumably need an updated extract1d datamodel to handle the new extension?  Not sure of the details here.
@stscijgbot-jp stscijgbot-jp changed the title Revised MRS 1d residual fringe correction to run automatically and populate x1d extension Revise MRS 1d residual fringe correction to run automatically and populate x1d extension Oct 24, 2024
@stscijgbot-jp
Copy link
Collaborator Author

Comment by Jane Morrison on JIRA:

David Law 
Here is what I am thinking on the logic.
Let ifu_rfcorr = False by default. If it is false then run extract 1d twice. First time no residual fringe correction, second time residual fringe correction.
I will update the datamodel to extract1d that will allow two spec_tables in output.
If the user has set ifu_rfcorr = True then they will expect the first extension to contain the residual fringe corrected data so only run extract1d once and  it will apply the residual fringe correction. The output will only have 1 spec_table 

Does that make sense  to you ? I was trying to keep it working the way it does now if the user has set ifu_rfcorr to True.
Of course I will need to make the documentation clear on what is going on.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Jane Morrison on JIRA:

Melanie Clarke Tyler Pauly after looking over the JP-3794: Add trace-based extraction option for NIRSpec and how the 'use_source_position' value is set None - I wonder if I could do something similar. 
Default if_rfcorr = None. If none then first run extract 1d with no rf correction and then run it a second time with rf correction. The output extract1d file will have 2 spec tables.
If ifu_rfcorr= True (user set) then only run extract 1d once and the output extract1d only has the rf spec table.
if ifu_rfcorr=False (user set) then only run extract 1d once and the output extract1d only has spec table without rf correction.
In both cases (True and False) a log message could be written to screen telling user of the None option. 

 

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Jane Morrison - rereading the ticket here and catching up, it sounds to me like what is requested is that we always run fringe correction and always provide two versions of the spectra.  The ifu_rfcorr parameter can be maintained for a while, but it will have no effect: we don't want the user to have control over this behavior, we want all data products to be consistent.  

David Law - can you confirm this interpretation?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Jane Morrison on JIRA:

David Law 
Melanie and I were discussing the cleanest way to add residual fringe information in the extracted data. Instead of adding an additional extension to the output data we were thinking we could instead add 3 columns to the extracted spec table that hold the residual fringe corrected flux, surface brightness and background. Users will need to update their code to read in these columns but they would need to update their code to read in the residual fringe corrected data anyway (which would be the second extension). Is adding these 3 columns acceptable to you instead of having two extension ?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Tyler Pauly on JIRA:

Would this use a new datamodel to capture the additional columns, or would this change the table structure for all extract_1d products?

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

We were thinking a new datamodel - MRSSpecTable, maybe, since it's unlikely this would be needed for any other modes.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by David Law on JIRA:

Jane Morrison Melanie Clarke That's right; for consistency we want to always do the same thing, providing two versions of the spectra.  The ifu_rfcorr parameter would ultimately go away, and simply be kept for a couple of builds to avoid crashing the pipeline if specified (instead printing out a warning message informing users of the new behaviour).  That does mean folks will need to get used to looking for the new values, but I think is probably less confusing in the long run.

Regarding adding columns instead of an extension, I think that sounds like a good modification.

A few additional clarifications based on the above:

  • The master_background step shouldn't need to be changed.  It uses x1d files from spec2, but we WANT to keep using the normal spectra without 1d fringe correction to subtract from the data cubes.
  • The spectral leak correction step will also need to be updated.  Currently it uses the FLUX extension of 1B to correct the FLUX extension of 3A, but we'll want to add using FLUX_RFCORR1D (or whatever we call it) from 1B to correct FLUX_RFCORR1D of 3A.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Jane Morrison on JIRA:

A few follow up questions. I pushed what I have so far to #9073 (so you can see how I am working this) 

David Law The 1-D residual fringe correction is best applied to "band" cubes. What about the output from Spec2 ? To see what would happen I ran extract 1d with ifu_rfcorr=True on cubes from spec2 and it failed.  So in that case do we want to keep the ifu_rfcorr = False ?
Also what should I do about a case where the ifucube is not a band cube. Say someone creates a channel cube - or even a multichannel cube  and then tries to run the extract1d on that data ? 

Melanie Clarke I created a new datamodel (see https://github.com/spacetelescope/stdatamodels/pull/376]) which creates a mrs_spectable. I need to resolve the problem that currently the ifu.py  ifu_extract1d assumes a multi spec model. Either I need update the multi spec model to accept an mrs_spectable or I need to update the mrs_spec.schema to hold the the basic slit information contained in multi spec.schema.yaml (slit name, slit_ra...) 
Do you have a preference ? I don't think ifu data will have multi spec tables so we could put the needed info in mrs_spec.schema. 

@stscijgbot-jp
Copy link
Collaborator Author

Comment by David Law on JIRA:

Hm, certainly I wouldn't expect it to get a meaningful correction for anything other than band cubes, though interesting that it actually crashes.

I'd be inclined to say that we don't even try running the rf code unless in from 'band' type cubes.  It should be possible to detect this looking at the CHANNEL and BAND header keywords; require CHANNEL='1', '2', '3', or '4' and BAND='SHORT', 'MEDIUM', or 'LONG'.  Anything with CHANNEL='12' or '34' or BAND='MULTIPLE' shouldn't be rf corrected, and in this case the pipeline should log an info statement about how RF correction won't be applied.

That in turn begs the question of what to do with the new rf_ columns in the Extract1d output if the rf correction hasn't been run.  We could duplicate the regular flux/surf_bright/etc extensions but that seems potentially misleading, and it may be better for them to be all-NaN values to make clear that the correction didn't get run.

'.

@stscijgbot-jp
Copy link
Collaborator Author

Comment by Melanie Clarke on JIRA:

Jane and I discussed this offline, but for anyone else following along, I'll look into the datamodels issue myself, for my own edification.  I'd like to experiment with a couple options for how to structure the schema.

@jemorrison
Copy link
Collaborator

I think if the cubes are not 'band' cubes then it might be best to set the 3 new columns to N/A so it clear the 1D residual fringe correction did not run

@stscijgbot-jp
Copy link
Collaborator Author

stscijgbot-jp commented Jan 16, 2025

Comment by Melanie Clarke on JIRA:

Datamodels support is in this PR: https://github.com/spacetelescope/stdatamodels/pull/377], now merged.

Assigning back to Jane for the jwst work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants