Skip to content

Electron-based instrument interface and OptikaInstrument - #18

Merged
roytsmart merged 13 commits into
mainfrom
feature/electron-based-instruments
Aug 2, 2026
Merged

Electron-based instrument interface and OptikaInstrument#18
roytsmart merged 13 commits into
mainfrom
feature/electron-based-instruments

Conversation

@roytsmart

@roytsmart roytsmart commented Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Draft: depends on unreleased optika changes on sun-data/optika #152 (the merged sensor PRs #191/#192/#193 are folded into it). Not mergeable until #152 lands in an optika release.

Makes the CTIS instrument interface electron-based and adds a real forward/inverse model backed by optika.

Interface

  • image() returns the electrons measured by the sensor; backproject() accepts electrons.
  • The per-instrument uncertainty callable is gone, replaced by an image(..., uncertainty=True) flag (see below).

IdealInstrument

  • Gains a quantum_yield parameter (electron/photon) and converts photons↔electrons around its regridding model.
  • Gains a read_noise parameter, applied once per readout after the wavelength integration.

OptikaInstrument (new)

  • Wraps an optika.systems.LinearSystem and delegates the forward/transpose — including the wavelength integration and read noise — to system.image_from_weights / backproject_from_weights, reusing the cached regridding weights/transpose so the operators aren't rebuilt on every call (important for the iterative MART solve).
  • Its sensor coordinates are in pixel units, matching the IdealInstrument (u.pix) convention.

Uncertainty

  • image(..., uncertainty=True) attaches the measurement-noise standard deviation as a NormalUncertainScalarArray.
  • Shot/Fano noise is computed per wavelength (exact, and correct for large EUV quantum yields) and summed in quadrature during the wavelength integration; read noise is applied once at the integrated readout — fixing a √N_wavelength over-count.
  • MartInverter weights its χ² with the attached width.

Tests

  • A shared AbstractTestAbstractInstrument suite (image, backproject, flux conservation, uncertainty, read-noise-once) runs for both IdealInstrument and OptikaInstrument.

All ctis tests pass locally (instruments + inverters).

🤖 Generated with Claude Code

@roytsmart roytsmart changed the title Electron-based instrument interface and LinearOptikaInstrument Electron-based instrument interface and OptikaInstrument Jul 24, 2026
@roytsmart
roytsmart force-pushed the feature/electron-based-instruments branch 2 times, most recently from dc1097c to a99bd94 Compare July 26, 2026 02:34
roytsmart and others added 8 commits July 30, 2026 11:31
@roytsmart @claude
Add electron-based instrument interface and LinearOptikaInstrument
e541bcc 
Make the CTIS instrument interface electron-based: `image()` returns the
electrons measured by the sensor and `backproject()` accepts electrons.

`IdealInstrument` gains a `quantum_yield` parameter and converts
photons<->electrons around its regridding model. `LinearOptikaInstrument`
wraps an `optika.systems.LinearSystem` and delegates to its
`image_from_weights`/`backproject_from_weights`, reusing the cached
regridding weights so the forward/transpose model (including the sensor's
`expose`/`photons_absorbed`) is not rebuilt each call.

`image()` gains an `uncertainty` flag: when set, it attaches the standard
deviation of the measurement noise as a `NormalUncertainScalarArray`. The
variance is computed per wavelength and summed in quadrature during the
wavelength integration, so it is exact even for the integrated image. The
MART inverter uses this to weight its chi-squared, replacing the old
per-instrument `uncertainty` callable.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@roytsmart @claude
Rename to OptikaInstrument and generalize the instrument tests
a1b2bf3 
Rename `LinearOptikaInstrument` to `OptikaInstrument` ("Linear" is already
implied by `AbstractLinearInstrument`; the distinguishing trait is that it
is backed by an `optika` system).

Generalize `AbstractTestAbstractInstrument` so the shared tests derive the
scene and coordinates from the instrument under test, and add
`TestOptikaInstrument` with a channel-aware `LinearSystem` fixture whose
scene grid spans the field of view. A new `test_backproject_conserves_flux`
asserts that re-imaging a backprojection preserves the total measured
electrons (the forward model's adjoint property) for every instrument.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@roytsmart @claude
Apply read noise once per readout in the instruments
fde855d 
Delegate the wavelength integration to optika: `OptikaInstrument` now
forwards `integrate` to `system.image_from_weights`/`backproject_from_weights`
(which sum over wavelength and apply the sensor's read noise once), instead
of integrating in ctis. This fixes the sqrt(N_wavelength) over-count that
resulted from the sensor applying read noise per wavelength.

`IdealInstrument` gains a `read_noise` field, applied once (Gaussian on the
nominal, in quadrature on the uncertainty) after its own wavelength
integration.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@roytsmart @claude
Test that read noise is applied once per readout
4b54058 
Add a shared `test_read_noise` (with a per-instrument `_with_read_noise`
hook) verifying that a nonzero read noise raises the integrated uncertainty
by exactly `read_noise` in quadrature, guarding against the
sqrt(N_wavelength) over-count.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@roytsmart @claude
Use pixel-coordinate distortion in the OptikaInstrument fixture
22f716c 
`LinearSystem.coordinates_sensor` is now in pixel units, so the test
fixture's distortion is expressed in arcsec/pix and nm/pix to match.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@roytsmart @claude
Add a unit parameter to backproject to match the scene units
f3210c9 
The forward model accepts a scene in photon or energy units, so let
`backproject` return the spectral radiance in whichever the caller requests,
so an inversion can be expressed in the same units as its input scene:

    instrument.backproject(image, unit=scene.outputs.unit)

`OptikaInstrument` forwards `unit` to optika's `backproject`, where the
wavelength and sensor physics live. `IdealInstrument`, a standalone analytic
model with no optika system, converts locally: its backprojection is in energy
units and is divided by the energy per photon when a photon unit is requested.
The default (`unit=None`) preserves the previous behavior of each instrument.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
@roytsmart @claude
Use SpectralPositionalVectorArray.volume_cell in _volume_scene
ffb21b3 
Replace the hand-written `wavelength.volume_cell(...) * position.volume_cell(...)`
(with the manual cell-center alignment) in `AbstractLinearInstrument._volume_scene`
with `coordinates.volume_cell(...)` from named-arrays 2.3. The scene coordinates
are coerced to a spectral-positional vector first so that `IdealInstrument`'s
Doppler scene also gets the method. Bump the named-arrays pin to ~=2.3.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
@roytsmart @claude
Remove accidentally-committed scratch files
801315a 
`na.py`, `demo.ipynb`, and `na.ipynb` are root-level scratch files that
were committed by mistake; they are not part of the `ctis` package.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
@roytsmart
roytsmart force-pushed the feature/electron-based-instruments branch from ed15346 to 801315a Compare July 30, 2026 18:07
roytsmart and others added 3 commits July 31, 2026 11:18
@roytsmart @claude
Use .spectral_positional to accept Doppler grids
7562d16 
Replace the hand-written `na.SpectralPositionalVectorArray(wavelength=...,
position=...)` coercions in `ctis.regrid` and `AbstractLinearInstrument.
_volume_scene` with the `.spectral_positional` property (named-arrays 2.4.0),
and bump the pin. Add `test_regrid_doppler`, covering a Doppler-grid regrid.
Behavior is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
@roytsmart @claude
Address review findings: remove stray test stub, robust band edges
4b0491c 
- Remove the empty `ctis/inverters/_results_test.py` that was committed
  by accident.
- `_integrate_wavelength`: collapse the wavelength coordinates to their
  min/max rather than the first/last edge, so the reported band edges are
  correct for a non-ascending wavelength grid.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
@roytsmart @claude
Compute IdealInstrument shot-noise width from the expected signal
9e2aeba 
When `noise=True`, the parent forward model returns a Poisson realization,
so deriving the shot-noise width from it made the attached uncertainty
depend on the noise draw (e.g. a pixel that realized zero photons got a
width of zero). Recompute the noiseless image for the width when a
realization was drawn, so the uncertainty is deterministic and matches the
`noise=False` case. Add a test asserting the width is unchanged by noise.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
@codecov

codecov Bot commented Aug 2, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (310231d) to head (8fd5f27).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##              main       #18    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           23        23            
  Lines          704       861   +157     
==========================================
+ Hits           704       861   +157
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

roytsmart and others added 2 commits August 2, 2026 10:05
@roytsmart
optika intersphinx
9391a8a
@roytsmart @claude
Cover the last uncovered lines in the instruments module
8fd5f27 
- Drop the dead `integrate` handling from `AbstractLinearInstrument.image`:
  both concrete subclasses override `image` and integrate themselves, and
  `IdealInstrument` always calls `super().image(integrate=False)`, so the
  parent branch was unreachable. The parent is now a per-wavelength photon
  forward model with integration left to the subclass.
- Add `test_axis_sensor_xy` and `test_backproject_scalar` to exercise
  `OptikaInstrument.axis_sensor_xy` and the bare-scalar `backproject` path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CtdKmedevWkDab6BWupXqQ
@roytsmart
roytsmart marked this pull request as ready for review August 2, 2026 18:24
@roytsmart
roytsmart merged commit d9a30b7 into main Aug 2, 2026
12 checks passed
@roytsmart
roytsmart deleted the feature/electron-based-instruments branch August 2, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@roytsmart