JP-3588: Use Pastasoss datamodel for NIRISS SOSS transform solution

[tapastro]

Resolves JP-3588

Closes #8398

This PR modifies the algorithm for NIRISS SOSS data transforms, used to align the data with the expected spectral trace positions. Previously, the ATOCA algorithm derived a 3 parameter rotation/offset transform that struggled to deal with certain atypical scenarios; this has been removed, and the https://github.com/spacetelescope/pastasoss team has provided an adapted version of their algorithm for use in the pipeline.

The pastasoss algorithm has introduced a new reference file type, aptly named pastasoss holding a PastasossModel. These files are not yet on the CRDS OPS server - regression testing to be done using CRDS TEST for now, with the expectation that any okify'ed results will need updating once OPS is mirrored to TEST and/or OPS' CRDS server code is updated to handle the new reftype.

Checklist for PR authors (skip items if you don't have permissions or they are not applicable)

• 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
• All comments are resolved
• Make sure the JIRA ticket is resolved properly

[codecov]

Codecov Report

Attention: Patch coverage is 18.46154% with 212 lines in your changes missing coverage. Please review.

Project coverage is 61.79%. Comparing base (3d7f8f5) to head (73fed8a).
Report is 11 commits behind head on master.

Files with missing lines	Patch %	Lines
jwst/extract_1d/soss_extract/pastasoss.py	21.60%	127 Missing ⚠️
jwst/extract_1d/soss_extract/soss_extract.py	12.50%	84 Missing ⚠️
jwst/extract_1d/extract_1d_step.py	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8763      +/-   ##
==========================================
+ Coverage   61.76%   61.79%   +0.02%     
==========================================
  Files         377      377              
  Lines       38743    38748       +5     
==========================================
+ Hits        23929    23943      +14     
+ Misses      14814    14805       -9     

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

[tapastro]

Started a run against CRDS-TEST only for test_niriss_soss.py: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1717/

[emolter]

Thanks for doing all this hard work Tyler! I know how challenging it is to spearhead merging someone else's code into our repo, and it looks excellent already!

Most of the comments are requests to remove unused things, plus a request to either fix the type hints to the satisfaction of mypy, or just remove them if that's easier. IMO it's better to have no type hints than ones that may not be correct.

Looking at the regression test failures, I am not surprised to see failures, but I was a bit surprised that there are 10 fewer HDUs in the output x1dints files than the truth files. Is that expected, and what is causing it? Is someone on the team tasked with ensuring that the output files look reasonable?

Because of the CRDS lookup errors with the pastasoss model, I couldn't check if the log messages look good/normal, but I didn't see anything that made me suspicious in the code itself, so that's probably fine.

This is beyond the scope of the PR, but it makes me vaguely uncomfortable that there are no unit tests for the entire suite. Do you think there are people we can push on in the SOSS team to make that happen in the future?

This is beyond the scope of the PR, but it makes me vaguely uncomfortable that there is no specific documentation page for the soss extraction, even though it's quite involved. Do you think there are people we can push on in the SOSS team to make that happen in the future?

[emolter]

perhaps replace hasattr with getattr to condense these lines

[tapastro]

Nice!

[tapastro]

Requesting re-reviews after addressing comments!

On the topic of unit tests and documentation: I requested these of the IDT members when they supplied the code, and it never came to fruition. I think it's reasonable to request a paragraph or two of documentation from the NIRISS team, with appropriate links to the pastasoss repo and to the ATOCA paper: https://iopscience.iop.org/article/10.1088/1538-3873/ac8a77. I can file a ticket requesting this.

I have a feeling that NIRISS will not be interested in supplying unit tests, though we can ask.

[emolter]

Still two unresolved comments from my last review. Otherwise this looks good to me and I'll hit the approve button once those are commented on or otherwise resolved

[melanieclarke]

It looks like there's a failure in the unit tests - a doctest for jwst.extract_1d.soss_extract.pastasoss.get_soss_traces.

I know the team is keen to get this change in this build, so I don't want to hold it up here, but I agree with Ned that this change could use some unit tests and documentation. I think some more code clean up will probably also be warranted, when the dust settles. Can we file a ticket for follow up in the next build?

[tapastro]

Yes, this bit of code is chock full of technical debt. I can file the ticket.

I had left some machinery in get_soss_traces that was left over from its use as a standalone repo, but I realize now that it no longer had any utility, so I've removed some of the offending code. I think it's in a functional state now? 😬

[melanieclarke]

Works for me, since the NIRISS SOSS team is happy with their testing.

[emolter]

@tapastro just a note that we should follow up with JP-3650 once this is merged, perhaps by changing its status to "Ready for Testing" and re-assigning it to someone else?