Skip to content

Run the source pipeline once per request instead of once per CCD - #2432

Merged
cquiroz merged 2 commits into
mainfrom
perf-pr/04-source-pipeline-once-per-request
Sep 24, 2026
Merged

cquiroz merged 2 commits into
mainfrom
perf-pr/04-source-pipeline-once-per-request

Conversation

@cquiroz

@cquiroz cquiroz commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

SEDFactory.calculate is split into a telescope stage, everything before the instrument, and an instrument stage that applies the components, the instrument background and, for the AO instruments, the AO system. We can cache those stage results.

GmosRecipe uses it so the three CCDs, the two IFU slits and the integration time second pass share one telescope stage.

Sharing only happens when sampling, observing range, bands, site and the trimmed sampling range match, so results are identical by construction.

This trims abour 10-15% of gnirs time

@cquiroz
cquiroz added this pull request to stack #2431 September 23, 2026 20:57
@cquiroz
cquiroz force-pushed the perf-pr/04-source-pipeline-once-per-request branch from d821e2d to 3f629af Compare September 23, 2026 22:30
Base automatically changed from sample-only-visible-range to main September 23, 2026 22:31
…t or order

SEDFactory.calculate is split into a telescope stage, everything before the
instrument, and an instrument stage that applies the components, the
instrument background and, for the AO instruments, the AO system. A per
request SourceCache memoizes the telescope stage per distinct sampling setup
and hands out a clone with the instrument stage applied on every call.

GmosRecipe uses it so the three CCDs, the two IFU slits and the integration
time second pass share one telescope stage. GnirsRecipe uses it so the six
cross-dispersed orders share one too: since the GNIRS wavelength fix the
recipe ran the whole pipeline per order, which had made cross-dispersed
requests about five times slower.

Sharing only happens when sampling, observing range, bands, site and the
trimmed sampling range match, so results are identical by construction.
@cquiroz
cquiroz force-pushed the perf-pr/04-source-pipeline-once-per-request branch from 3f629af to ec1cd64 Compare September 23, 2026 22:31
@mergify

mergify Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Refactors ITC source calculations into reusable telescope and instrument stages, allowing GMOS and GNIRS requests to share telescope-stage results across CCDs, slits, and passes.

Changes:

  • Adds telescope-stage caching in SEDFactory.SourceCache.
  • Updates GMOS and GNIRS recipes to use cached source results.
  • Clones cached spectra before applying instrument-specific processing.
File Description
bundle/​edu.gemini.itc/​src/​main/​java/​edu/​gemini/​itc/​gnirs/​GnirsRecipe.java Updated as part of this pull request.
bundle/​edu.gemini.itc/​src/​main/​java/​edu/​gemini/​itc/​gmos/​GmosRecipe.java Updated as part of this pull request.
bundle/​edu.gemini.itc/​src/​main/​java/​edu/​gemini/​itc/​base/​SEDFactory.java Updated as part of this pull request.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

atTelescope validates the source against getFirstNonZero and
getLastNonZero, so two instruments that differ there must not share a
cached telescope stage. GMOS CCDs share both values, so outputs and
cache hits are unchanged.

@toddburnside toddburnside left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems fine to me.

@cquiroz
cquiroz merged commit 6db68e6 into main Sep 24, 2026
4 checks passed
@cquiroz
cquiroz deleted the perf-pr/04-source-pipeline-once-per-request branch September 24, 2026 17:37
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.

3 participants