AMI3 post-commissioning updates

[stscijgbot-jp]

Issue JP-3153 was created on JIRA by Rachel Cooper:

A number of changes to the standalone algorithm that AMI3 is based on (ImPlaneIA) have been made since 2020. The goal for these updates is to be able to reproduce commissioning results obtained with ImPlaneIA. Other improvements will be introduced later.
Inputs:
The default input to the AMI3 stage should be calints.fits files. In the case where a 2D image (cal file) is supplied, the code should automatically expand the dimensions (e.g. (80,80) --> (1,80,80)) (https://jira.stsci.edu/browse/JP-1714). Calints files are not currently produced automatically for AMI exposures; this is a prerequisite to these updates (see https://jira.stsci.edu/browse/JP-2904) 
Outputs:
Instead of the current ami_analyze FITS output, we will be moving to industry-standard OIFITS format (see https://jira.stsci.edu/browse/JP-1713) There will be two types of OIFITS files produced from a single calints.fits file input:

• Observables averaged over all integrations (input for subsequent steps)
• Observables for each integration (users can perform their own more detailed analysis)

These file formats will be nearly identical to each other except for the shapes of table extensions. We would also like there to be a third new output from the ami_analyze step:

• FITS file containing the following extensions:
  ** CENTER: The 3-D centered, cropped, bad-pixel-fixed input image
  ** N_CENTER: Normalized 3-D centered, cropped, bad-pixel-fixed input image
  ** FIT: A 3-D image of the fitted model.
  ** N_FIT: A normalized 3-D image of the fitted model.
  ** RESID: A 3-D image of the fit residuals.
  ** N_RESID: A normalized 3-D image of the fit residuals. 

Subsequent steps:

In general, we do not average together multiple science exposures. Users may sometimes wish to have multiple calibrator star exposures combined. Whether/how this step can be run automatically using associations is TBD; we do not want users combining calibrators without first examining their quality individually. For this update, we would like to turn off this step for now. The step will eventually need to be updated to take the new (integration-averaged) OIFITS output from ami_analyze and produce new averaged OIFITS files.
ami_normalize will be updated to take the integration-averaged science OIFITS files from ami_analyze and either an integration-averaged calibrator OIFITS file or an exposure-averaged calibrator OIFITS file. The final output of ami_normalize will be a single calibrated OIFITS file. (multi-integration OIFITS files cannot be averaged together or normalized because target and calibrator typically have a different number of integrations)

Algorithm Improvements

At the beginning of the ami_analyze step, a bad pixel cleaning sub-step will be run. The datamodel will only be updated in-memory for now. A number of optional keyword arguments will be added for ami_analyze to replicate some of the flexibility currently in the standalone algorithm:

• src: source spectral type to use when creating the model
• usebp: if True, exclude pixels flagged DO_NOT_USE in DQ array from fitting. If False, use all pixels regardless of DQ array.
• firstfew: analyze only the first N integrations of the data cube
• chooseholes: fit only certain fringes, identified by segment hole names
• bandpass: allow user to pass a bandpass, either a Synphot spectrum object or appropriately shaped numpy array.

Other changes:

• https://jira.stsci.edu/browse/JP-2869 Revert to use standalone MFT rather than Poppy due to sign convention change
• Replace use of throughput reference file (too coarsely sampled) with combined filter and source bandpass using WebbPSF, stsynphot.
• Make the centering of individual integrations less sensitive to CR hits in the subarray by centering around the commanded integer pixel position, calculated from header keywords.

[stscijgbot-jp]

Comment by Alicia Canipe on JIRA:

Hi Rachel Cooper! Just for tracking (I know you all got too busy to get it in for 9.3, but here's the tentative timeline from DMS for the next build, which is actually Build 10.0):

• Build 9.3
  ** INS algorithms/changes were due by 26 May 2023
  ** SCSB must complete development by 21 June 2023 (this was moved up a week to accommodate 9.3 regression testing) so it can be turned over to I&T on 20 July 2023
• Build 10.0
  ** DMS shifts the number according to the next cycle, which is Cycle 2 in this case
  ** INS algorithms/changes must be in by ~end of August 2023
  ** Based on an August algorithm deadline, SCSB must complete development late Sept/early Oct. so it can be turned over to I&T Oct. 19th

[stscijgbot-jp]

Comment by Alicia Canipe on JIRA:

One more note (CC Stephanie La Massa and Anton Koekemoer /Greg Sloan ): when NIRISS is ready, it may be worth a tag-up w/ CalWG leads and SCSB to make sure everyone is on the same page about changes being requested. There are a lot of items noted in this ticket, and the linked tickets have a "low" priority assignment from NIRISS. I just want to make sure we don't miss something if this needs to get into the next build. 

Other groups may be affected, e.g., I'm not sure if MAST folks or anyone else would need to be looped in about the OIFITS change being requested, JDox updates will be needed, etc. 

Howard Bushouse FYI. 

[stscijgbot-jp]

Comment by Anton Koekemoer on JIRA:

Thanks Alicia Canipe, I agree this would be good to schedule a discussion between CalWG, SCSB and NIRISS to go over the proposed algorithm changes, once NIRISS is ready.

Since it's quite specific to AMI and not impacting the broader CalWG, we could consider scheduling this in an "out-of-cycle" slot, ie not one of the regular full monthly CalWG meetings, but another Tuesday instead. Some options are Tue Jun 27, Jul 18, Jul 25, or later (all would be at 11am ET).

It would be good to know from SCSB who might be available to help answer Rachel Cooper's questions about datamodels in the meantime, so I've also added Nadia Dencheva  to this ticket to provide further support there.

[stscijgbot-jp]

Comment by Kevin Volk on JIRA:

There was discussion of this ticket in the Cal-WG meeting on 1 August, and a nominal deadline of 1 September for the JWST pipeline pull request (since Rachel is working on the code changes in a github branch according to what we were told by SCSB in the meeting) is 1 September, in order for things to be in place by 15 September when the pipeline version goes to DMS for testing.  Rachel was not at the meeting so I have sent an e-mail to her alerting her to the schedule in addition to making this comment.

[stscijgbot-jp]

Comment by Stephanie La Massa on JIRA:

Commenting on this ticket to note that NIRISS is ready for this update to be implemented into build 10.0. Rachel Cooper opened a draft PR here: #7862

Rachel also mentioned she's waiting for some feedback from SCSB about final minor changes to make, but I wanted to make sure this work was visible on this ticket to confirm we're on track for getting these updates into build 10.0?

Do we need to schedule a meeting with MAST folks to make sure the archive can serve OIFITS files? Are there any other blockers in getting this update deployed in build 10.0? 

[stscijgbot-jp]

Comment by Rosa Diaz on JIRA:

I was not aware of the request to generate a new type of files or see an archive ticket to define the new type of file. I will investigate from my side and let you know, though, I am afraid it might be too late for build 10.0. But the archive has builds ~ every month, so it might be possible to add it.

[stscijgbot-jp]

Comment by Howard Bushouse on JIRA:

Rachel Cooper Just wondering about the status of the big PR #7862 that implements most of this. It's still marked as being in Draft form. Any idea of how close it is to be done and ready for review and testing? Trying to determine whether we can try to include this in DMS B10.0, for which we need to finalize development by 15-Sep-2023.

[stscijgbot-jp]

Comment by Rachel Cooper on JIRA:

Hi Howard Bushouse, I'm actively working on the last few changes I need to make to get the full calwebb_ami3 pipeline running with the updated steps before I convert the draft PR to be open, but I need a bit more help on changes to some of the parts of the library I'm less familiar with, namely the association generator and making sure the products get saved with the correct suffixes. Rosa Diaz also mentioned I would need to file an archives ticket for the changed data products. I'm still hoping to get this in in time for build 10.0 but I realize I was unaware of these external updates that need to happen, so I understand if that's not feasible with the other work your team(s) have. 

[stscijgbot-jp]

Comment by Howard Bushouse on JIRA:

Rachel Cooper I can help with the ASN generator updates. Are these the changes to just have AMI processing switchover from using rate and cal products to rateints and calints products? Hooks for those changes are already in the ASN rules, but commented out, so that change should be simple to make. Was there anything in addition to that for the ASN generator?

What are the new product types and their file name syntax? I could help steer you in the right direction for those too.

[stscijgbot-jp]

Comment by Rachel Cooper on JIRA:

I already uncommented the ASN changes for the 3D inputs, but I thought now that there is no intermediate "amiavg" product (since we are skipping the ami_average step) and each exposure will be calibrated by a matching psf reference exposure something would need to change there, but I haven't actually dug into the details yet. I put some details in my latest comment on this ticket: https://jira.stsci.edu/browse/JP-1713 but I've copied the relevant info here.
The new files are:
From ami_analyze: _ami.oifits (averaged over integrations), _amimulti.oifits (multi-integration), and _amilg.fits. The first two use the new AmiOIModel datamodel, and the last is the AmiLGFitModel. 
From ami_normalize:
_aminorm.oifits

[stscijgbot-jp]

Comment by Rosa Diaz on JIRA:

I filed   JWSTDMS-918 so DMS can start looking into getting these files into the system, including the archive and other subsystems that need to know about it.

NIRISS can start thinking about the metadata that needs to be added to the archive  db in order to support search and retrieval of these files.

[stscijgbot-jp]

Comment by Anton Koekemoer on JIRA:

hi all, just checking on the status of this – can Howard Bushouse  and Rachel Cooper  post here please what the next steps are that are needed for this to be implemented, and who that is assigned to, or could you organize further discussion if needed and mention that here too? Thanks!

[stscijgbot-jp]

Comment by Rachel Cooper on JIRA:

Sorry, I must have missed the comment notification on this ticket. Anton Koekemoer Howard Bushouse I was under the impression that folks from MAST were going to set up a meeting to determine if it will be possible to use the .oifits file extension. Following that decision, I will need to make some final documentation updates to use the new file suffixes and extensions. I gave the requested information about these suffixes again on https://jira.stsci.edu/browse/JWSTDMS-918
and haven't heard anything further. I will follow up on the scheduling of the meeting on that ticket.

[stscijgbot-jp]

Comment by Anton Koekemoer on JIRA:

thanks Rachel Cooper for the update, greatly appreciated, and by all means feel free to use JWSTDMS-918 as the place to follow up for the next steps.

[stscijgbot-jp]

Comment by Rachel Cooper on JIRA:

Hi Howard Bushouse or Brett Graham,

Since we've come to a consensus on the file suffixes & extensions for the outputs of the AMI3 steps, I'd like to make sure I get the necessary code changes are in place when the deadline for the next build (10.2?) rolls around. Could you or another pipeline savant please help me identify where (in jwst/lib/suffix.py?) I need to make changes so that the outputs of each step get the appropriate name? For reference, we concluded that those will be:

ami_analyze:

_ami-oi.fits

_amimulti-oi.fits

_amilg.fits

ami_normalize:

_aminorm-oi.fits

Or, when running the full stage with an association file, the reference PSF exposure gets the suffix _psf-ami-oi.fits.
Currently, the three outputs from the ami_analyze step automatically get named with suffixes _0_amianalyzestep.fits, _1_amianalyzestep.fits, _2_amianalyzestep.fits.

Thanks!

[stscijgbot-jp]

Comment by Brett Graham on JIRA:

Hi Rachel,

Howard pointed me towards a recent PR of his adding a suffix:
https://github.com/spacetelescope/jwst/pull/8111/files#diff-18af94cf2c780646c7043d3101aa6551385ca90326193d361ddd502b4205f453R152
I think adding the new suffixes to this list and modifying the step to use the new suffixes (by passing them into call of save_model) might work (this is new to me so it might require some back-and-forth to get everything working).
Let me know if it's helpful to make a PR against a branch with these changes and if so please let me know which branch to use as the base.
Also let me know if there's anything I can do to help!

[stscijgbot-jp]

Comment by Howard Bushouse on JIRA:

It looks to me like many (or all?) of the new suffixes will replace existing ones that are listed in SUFFIXES_TO_ADD at https://github.com/spacetelescope/jwst/blob/master/jwst/lib/suffix.py#L38. This is the list of suffixes for standard/normal products that come out of every pipeline run and are produced by default. So in this case it looks like ami will be replaced by amilg, ami-oi, and amimulti-oi, and aminorm will be replaced by aminorm-oi. Meanwhile amiavg will go away.

Not sure what kinds of entries might need to be added to the calculated_suffixes list at https://github.com/spacetelescope/jwst/blob/master/jwst/lib/suffix.py#L60. Those are the names of products coming from individual steps when they get run standalone. Sometimes the best way is to let the software figure it out for you by running the find_suffixes function that's in the jwst/lib/suffix.py module.

[stscijgbot-jp]

Comment by Rachel Cooper on JIRA:

Thank you! I have replaced the suffixes in SUFFIXES_TO_ADD with the new ones, and had figured out that I can at least use the suffix argument in the save_model call in the calwebb_ami3 pipeline script. For the case when the steps are run individually though I'm still unclear on how the code picks which of the suffixes in the set of _calculated_suffixes to use. There's no explicit call to save_model in the top level of the step (e.g. ami_analyze_step.py), though the products do seem to get saved by default and are numbered the way I mentioned. I also see both 'amianalyzestep' and 'ami_analyze' as suffixes in the calculated suffixes list and it's not clear to me when/if just 'ami_analyze' is used as opposed to 'amianalyzestep' for the suffix. I'm also not sure if there's a precedent for a single step returning multiple output products and how that should be handled; I see some steps have it as a default in the spec block and/or assign a .suffix attribute but it seems to vary and since there are three for the amianalyze step I'm not sure how that would work. 

[braingram]

I'm not familiar with this and looking into it the code is quite confusing so I could very well be getting most or all of this wrong.

The step code in stpipe: https://github.com/spacetelescope/stpipe/blob/9e7c4416fa8776c00843c18390a1dc2d7b4179d0/src/stpipe/step.py#L537
will save results returned by run (if an output file is provided or save_results is set to true, I'm not sure if either of these are set when running the step individually. To confirm, are these individual step runs through the command line or creating the python class and running it in a script?). make_output_path appears to do the bulk of determining the file name used for saving the result(s): https://github.com/spacetelescope/stpipe/blob/9e7c4416fa8776c00843c18390a1dc2d7b4179d0/src/stpipe/step.py#L1020 with the suffix either:

• provided as an argument (this appears unused for the automatic result saving)
• fetched from the step.suffix attribute
• calculated from the step.name

If I'm understanding your needs (please correct me if I'm mistaken), you're looking to have a step save several files (with the contents from a list of results returned from the process call) each with a different suffix. I think this might be possible by overriding make_output_path (which receives the index idx of the result being saved) to change the suffix for each saved item.

[rcooper295]

Hi Brett, sorry for the delay. I've also had a hard time understanding what's going on in this particular bit of the code but your explanation is very helpful. I generally don't run the pipeline steps or stages directly from the command line, I usually set up the class and run it in a notebook or script but ideally either way would produce the same results.

Your understanding is correct, I'm looking to have three outputs returned when the ami_analyze step is called (as a list or tuple or whatever makes sense) and also saved with their individual suffixes when save_results = True.

[stscijgbot-jp]

Comment by Rachel Cooper on JIRA:

PR-7862 has been opened for review.

[stscijgbot-jp]

Comment by Howard Bushouse on JIRA:

Fixed by #7862

[stscijgbot-jp]

Comment by David Law on JIRA:

Rachel Cooper Rachel Plesha Another AMI-related testing issue for you to sign off on.

[stscijgbot-jp]

Comment by Rachel Plesha on JIRA:

Rachel Cooper can this ticket be set to Done or is there more testing still now that the build is officially installed?