Skip to content

Fix readnoise pixfrac scaling#350

Merged
schlafly merged 3 commits into
spacetelescope:mainfrom
schlafly:fix-readnoise-pixfrac-scaling
Apr 17, 2026
Merged

Fix readnoise pixfrac scaling#350
schlafly merged 3 commits into
spacetelescope:mainfrom
schlafly:fix-readnoise-pixfrac-scaling

Conversation

@schlafly

Copy link
Copy Markdown
Collaborator

Nikhil and Greg's investigations of the romanisim L3 files indicated that they were not as deep as they should be. The root cause is that the read noise scaling with the pixel scale fraction was missing a factor of the square of the pixel scale fraction.

The default read noise computation uses the read noise per read and imagines that the total read noise is just sqrt(2) times the per-read read noise, a pessimistic assumption. It then converts that to electrons / s using the total exposure time times the square of the pixscalefrac, reasoning that the total exposure time needs to be split up among pixscalefrac ** 2 output pixels. The result is then converted to MJy / sr units for use downstream.

The final conversion to MJy / sr units assumed that we were starting with e / s / coadd pixel units, but the computation had been done assuming e / s / native pixel units, introducing a spurious factor of the pixscalefrac squared. This PR fixes that issue.

It also fixes an unrelated error where the effective read noise was zeroed if an explicit value was sent by the user, makes some mild documentation updates, and adds a test of the read noise scaling with pixel scale fraction.

Closes #349 .

@schlafly

Copy link
Copy Markdown
Collaborator Author

@npadmana , mind taking a look at this and seeing if it addresses the issues you found?

@codecov

codecov Bot commented Apr 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.26%. Comparing base (d18949b) to head (2543ec6).
⚠️ Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #350      +/-   ##
==========================================
- Coverage   89.63%   89.26%   -0.37%     
==========================================
  Files          17       17              
  Lines        2633     2664      +31     
==========================================
+ Hits         2360     2378      +18     
- Misses        273      286      +13     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@npadmana

Copy link
Copy Markdown
Contributor

I took a very quick look at this.

This significantly improves the detections, although the pessimistic/default case seems to be worse than what I get with a single L2 mapping directly to an L3. My understanding is that difference comes from the up the ramp reads, while the default version assumes 2 reads. Is that right?

@schlafly

Copy link
Copy Markdown
Collaborator Author

I think that's part of it. It's certainly true that the assumption we're making for the default effective read noise is the maximally pessimistic one.

The larger issue is probably with how the resampling is simulated. For the single image case, pixfrac = 0.5 case, romancal invents 4 pixels for each single input pixel. The errors accordingly get correlated.

What I'm trying to simulate (modulo more bugs) in romanisim is that in your nominal N seconds you have taken 4 exposures each of N/4 in length and interleaved them. So your uncertainties are uncorrelated but you pay more read noise. I think that for a single exposure you wouldn't really want to use pixscalefrac = 0.5. I'll need to think more about what I would expect the noise to look like for the single exposure romancal case.

@schlafly schlafly marked this pull request as ready for review April 16, 2026 18:46
@schlafly schlafly requested a review from a team as a code owner April 16, 2026 18:46
@schlafly schlafly merged commit 387adb5 into spacetelescope:main Apr 17, 2026
27 of 28 checks passed
@schlafly schlafly deleted the fix-readnoise-pixfrac-scaling branch April 17, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--effreadnoise CLI option is silently ignored

2 participants