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

Adjust saturation threshold depending on the read pattern #836

Merged
merged 10 commits into from
Sep 20, 2023

Conversation

schlafly
Copy link
Collaborator

@schlafly schlafly commented Aug 16, 2023

Resolves RCAL-618

This PR in concert with spacetelescope/stcal#188 updates the saturation threshold to depend on the read pattern.

The idea is that the saturation reference file gives the number of counts at which a given pixel saturates. Roman downlinks resultants, which are usually averages of many reads. The current saturation flagging checks if a resultant is above the saturation threshold, but we want to know if any read in the resultant is above the threshold. The resultants in a sense dilute the saturated signal---a saturated last read is averaged with unsaturated earlier reads, bringing the resultant value beneath the saturation threshold. This PR corrects for that affect by passing stcal an additional argument, read_pattern, that specifies how reads are allocated to resultants, and using that information to adjust the saturation threshold resultant-by-resultant.

Checklist

  • added entry in CHANGES.rst under the corresponding subsection
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)

@schlafly schlafly added enhancement New feature or request RCAL Saturation labels Aug 16, 2023
@github-actions github-actions bot added Photom testing dependencies Pull requests that update a dependency file labels Aug 16, 2023
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 16, 2023
@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Patch has no changes to coverable lines.

Files Changed Coverage
romancal/saturation/saturation.py ø

📢 Thoughts on this report? Let us know!.

@schlafly
Copy link
Collaborator Author

Regression tests are failing for unrelated reasons (presumably the forthcoming release?), though more broadly I think we expect the change to the saturation algorithm to introduce some differences in the saturation flagging.

@schlafly
Copy link
Collaborator Author

I think this is good to go, though I doubt we actually want to merge it until the corresponding stcal PR is merged.

@schlafly schlafly marked this pull request as ready for review August 17, 2023 15:08
@schlafly schlafly requested a review from a team as a code owner August 17, 2023 15:08
Copy link
Collaborator

@ddavis-stsci ddavis-stsci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but of course the real work is in stcal.

@ddavis-stsci ddavis-stsci added this to the 24Q1_B12 milestone Aug 29, 2023
@github-actions github-actions bot removed the dependencies Pull requests that update a dependency file label Sep 20, 2023
@schlafly schlafly merged commit 8880382 into spacetelescope:main Sep 20, 2023
@schlafly schlafly deleted the saturation-dilution branch September 20, 2023 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request Photom RCAL Saturation testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants