Skip to content

Revive I3S spot matching in the live scan (one scan runs Groth + I3S)#1611

Open
JasonWildMe wants to merge 1 commit into
mainfrom
fix/i3s-regression-live-scan
Open

Revive I3S spot matching in the live scan (one scan runs Groth + I3S)#1611
JasonWildMe wants to merge 1 commit into
mainfrom
fix/i3s-regression-live-scan

Conversation

@JasonWildMe

Copy link
Copy Markdown
Collaborator

Problem

The React SpotMappingCard (10.10) advertises "Modified Groth and I3S" and renders an "I3S scan results" link, but I3S results are never produced — a regression.

When the spot-matching scan was rewritten as the synchronous GrothMatchServlet (the "Optimized Modified Groth" work), only Modified Groth was ported. I3S stayed in the now-dead async grid path (ScanWorkItem.execute), which is never instantiated. Everything else still works — the I3S algorithm (EncounterLite.i3sScan), the viewer (encounters/i3sScanEndApplet.jsp), and the API flags (Encounter.spotMappingJsonForApiGet() checks for lastFull[Right]I3SScan.xml) — but nothing writes the I3S result file, so resultsI3SLeft/Right is always false and the link never appears.

Fix

Have the single live scan produce both result files, mirroring the old ScanWorkItem.execute combine pattern:

  • GrothMatchServlet Phase 1 builds the query EncounterLite (the input i3sScan needs).
  • Phase 2 runs el.i3sScan(queryLite, rightScan) alongside Groth on each candidate and stores the I3S score + point-map on the same MatchObject via setI3SValues. Gated on the scan-side spots being present; no reference-spot requirement (the fiducial-point helpers fall back to spot bounds when reference spots are absent, matching the legacy behavior). Wrapped per-candidate so one bad candidate can't abort the scan. The exact EncounterLite each match scored against is captured (scannedI3SLites) so the writer indexes the same spot order even if the live matchGraph entry is replaced mid-scan.
  • Phase 3 writes lastFull[Right]I3SScan.xml after the Groth XML. The existing UI link lights up automatically.

New org.ecocean.grid.I3SResultWriter produces the I3S XML in the shape i3sScanEndApplet.jsp expects. Notable correctness details (caught in review):

  • Filters valid matches (0.001 < score ≤ 2.0) before the 100-match cap. NewI3SMatchComparator sorts ascending and unscored candidates default to 0, so a cap-then-filter would fill the cap with zeros and yield an empty file.
  • Sources spot coordinates from the captured EncounterLites (query + catalog), not a DB re-fetch — so Pair.getM1()/getM2() indices always line up with the spot order I3S actually compared.
  • Builds each <match> detached and attaches it only after both <encounter> elements are fully populated, so a per-match failure can't leave a one-encounter <match> that breaks the viewer.
  • Closes the writer in a finally.

getPointsForBestMatch and i3sScan were both confirmed non-mutating on the shared EncounterLite, so no spot reset is needed between them.

Trade-off

Running both algorithms in one synchronous scan ~doubles per-candidate compute over the catalog. Accepted (single "Start Scan" produces both, matching the UI); could move to async later if scan latency becomes a problem.

Verification

🤖 Generated with Claude Code

When the spot-matching scan was rewritten as the synchronous GrothMatchServlet
("Optimized Modified Groth"), only Modified Groth was ported. I3S was left in the
now-dead async grid path (ScanWorkItem.execute, never instantiated), so the I3S
result file is never produced and the "I3S scan results" link in the React
SpotMappingCard never appears -- a regression. The I3S algorithm
(EncounterLite.i3sScan), the viewer (i3sScanEndApplet.jsp) and the API flags
(Encounter.spotMappingJsonForApiGet checks lastFull[Right]I3SScan.xml) all still
work; only the producer was missing.

Have the single live scan produce both results, mirroring the old ScanWorkItem
combine pattern:

- GrothMatchServlet Phase 1 builds the query EncounterLite (i3sScan input).
- Phase 2 runs el.i3sScan(queryLite, rightScan) alongside Groth on each candidate
  (gated on scan-side spots present; fiducial points fall back to spot bounds when
  reference spots are absent, matching legacy; wrapped so one bad candidate can't
  abort the scan), and stores the I3S score/point-map on the same MatchObject via
  setI3SValues. The exact EncounterLite each match scored against is captured so the
  writer indexes the same spot order even if the matchGraph entry is replaced.
- Phase 3 writes lastFull[Right]I3SScan.xml after the Groth XML; the existing UI
  link lights up automatically.

New org.ecocean.grid.I3SResultWriter produces the I3S XML in the shape
i3sScanEndApplet.jsp expects. It filters valid matches (0.001 < score <= 2.0)
BEFORE the 100-match cap (the comparator sorts ascending and unscored candidates
default to 0, so cap-then-filter would yield an empty file), sources spot
coordinates from the captured EncounterLites (no DB re-fetch / no index mismatch),
builds each <match> detached and attaches it only after both encounter elements are
fully populated, and closes the writer in a finally.

getPointsForBestMatch and i3sScan were both confirmed non-mutating on the shared
EncounterLite, so no spot reset is needed between them.

Note: running both algorithms in one synchronous scan ~doubles per-candidate
compute over the catalog. Reviewed with codex to convergence.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.50%. Comparing base (e1fcd4e) to head (ac4d064).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1611   +/-   ##
=======================================
  Coverage   51.50%   51.50%           
=======================================
  Files         308      308           
  Lines       12100    12100           
  Branches     3808     3897   +89     
=======================================
  Hits         6232     6232           
+ Misses       5585     5578    -7     
- Partials      283      290    +7     
Flag Coverage Δ
backend 51.50% <ø> (ø)
frontend 51.50% <ø> (ø)

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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants