Enable and fix the TGO CaSSIS driver#720
Conversation
Re-enable the TGO CaSSIS driver (was disabled) and fix two bugs that
broke it: ephemeris_start_time must unpack utcToEt()[0], and CaSSIS
SummingMode is an enum (0=1x1, 1=2x2, 2=4x4) not a factor, so emit the
ISIS-style summing (sumMode*2, or 1) rather than the raw 0 (a 0 makes the
USGSCSM frame model non-self-consistent).
Add the CaSSIS rational ratio-of-quadratics distortion (Tulyakov/Ivanov,
EPFL; the model ISIS uses in TgoCassisDistortionMap): a CassisDistortion
mix-in packing the 36 INS<ikid>_OD_A{1,2,3}_{CORR,DIST} coefficients, the
driver using it, the CASSIS enum value, and cassis parsing in Util.cpp.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In NaifSpice.sensor_position, the LT+S + correct_lt_to_surface branch computes adjusted_time (= ephem - obs_tar_lts + radius_lt) but then queried the target body position (ssb_tars) at the raw ephemeris time. The body moves along its orbit during the surface light time, leaving a constant camera-center bias (tens of meters) versus ISIS. Sample ssb_tars at adjusted_time. General fix, affects any sensor with LT_SURFACE_CORRECT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ISIS uses 0.5-based CCD coordinates (pixel centers at half integers) while CSM is 0-based. The CaSSIS driver took the IK boresight directly, leaving the CSM look offset from ISIS by half a pixel in each of sample and line, i.e. sqrt(0.5^2 + 0.5^2) ~ 0.707 px. Subtract 0.5 from detector_center_sample and detector_center_line, as the LRO, MRO, Dawn, MESSENGER, MEX, Kaguya and KPLO drivers already do. cam_test ISIS-vs-CSM then drops from 0.707 px to ~7e-4 px. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the surface light-time fix out of the shared NaifSpice.sensor_position and into a TGOCassisIsisLabelNaifSpiceDriver.sensor_position override, so it applies to CaSSIS only and leaves the shared path unchanged for every other sensor. The override is the shared LT+S surface-correction branch with the one change that the target body is sampled at the surface-light-time adjusted time; CaSSIS is a single-record framer so this is exact. Behavior and results are unchanged for CaSSIS (cam_test ISIS-vs-CSM ~7e-4 px). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Un-xfail test_cassis_load now that the driver works, and regenerate the golden cassis_isd.json (summing 1, the CaSSIS distortion, the half-pixel detector center). Fix test_ephemeris_start_time to mock utcToEt returning a tuple (it returns (et, kernels) now). Add honest unit tests for sample/line_summing (the SummingMode enum) and detector_center_sample/line (the ISIS 0.5-based to CSM 0-based conversion). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Generate the test ISD with attach_kernels False (as the MEX test does) so the machine-specific absolute kernel paths are not written into the committed golden cassis_isd.json. Regenerate the golden without them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The real-kernel *_load tests across many missions began failing once CI picked up spiceql 1.4.0 (environment.yml had spiceql>=1.3.0 unpinned and the CI env is rebuilt fresh each run). With searchKernels=False and useWeb=False the tests rely entirely on the committed per-image kernels, which 1.4.0 no longer resolves into the planetary ephemeris and CK frames the way 1.3.x did. Pin to <1.4 to restore the green test environment. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The CI failures are not from this PR. A recent spiceql release (1.4.0) breaks the real-kernel *_load tests repo-wide. environment.yml had spiceql unpinned, so CI started pulling 1.4.0 (released after main last passed). Under 1.4.0 the committed per-image kernels no longer resolve (SPKINSUFFDATA, "ck rotation ... can not be found") across many missions (apollo, voyager, mex, lro, and others); the test furnishing uses searchKernels=False and useWeb=False, so there is no fallback. The mocked tests are unaffected. This hits main and every open PR, not just this one. Commit bf72317 here pins spiceql to >=1.3.0,<1.4 makes the PR pass. |
The CASSIS distortion type added here references ale::DistortionType::CASSIS, which lives in the companion ALE PR (DOI-USGS/ale#720) and is not yet on ale main, so the build fails with "CASSIS is not a member of ale::DistortionType". Point the ale submodule at oleg-alexandrov/ale cassis_support so CI can build and test. This must be reverted to ale main once ale#720 is merged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| - nlohmann_json | ||
| - numpy | ||
| - spiceql>=1.3.0 | ||
| - spiceql>=1.3.0,<1.4 |
There was a problem hiding this comment.
1.4.1 fixed the bugs, you can unpin.
There was a problem hiding this comment.
Unpinned. Tests pass.
Done with Claude/AI assistance.
spiceql 1.4.1 restores the per-image kernel furnishing that 1.4.0 broke, so the <1.4 cap added earlier is no longer needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Tests pass with fixed SpiceQL. Pin to prev version of this removed. |
Im unfamiliar with TGO Cassis processing, but is this a required step using ISIS-style (not CSM) cameras? Or is it more about how ESA is managing things? Should we document this on the asc-public-docs? Maybe something I can bring up with my meetings with the TGO folk. |
|
It's not a CSM-vs-ISIS thing. It is about which CaSSIS product you start from, and it affects the ISIS-native camera just as much as CSM. The tgocassis2isis ingester has two label translations. For the instrument-team products that carry the full CaSSIS_Header and FSW_HEADER, TgoCassisInstrument.trn maps the FSW exposure timestamp into SpacecraftClockStartCount automatically, so there is nothing to do. For the calibrated products exported to the ESA PSA, the em16_tgo_cas namespace files you download from the archive, the fallback translation TgoCassisExportedInstrument_PSA.trn fills only StartTime and has no SpacecraftClockStartCount group, so the cube comes out without it. That keyword is required on the cube regardless of camera model. The ISIS TgoCassisCamera constructor reads SpacecraftClockStartCount at TgoCassisCamera.cpp lines 63-65, and spiceinit builds its cache through the camera, so both fail if the keyword is missing. ALE and CSM are downstream of all this and are unaffected. The editlab step is therefore a cube-side ISIS ingest matter, not anything specific to CSM. It is worth noting that the value itself is not used for the geometry. The camera reads the keyword but computes the time from StartTime in UTC, per the long-standing TODO in TgoCassisCamera.cpp. So the keyword has to be present, but its value is effectively unused for timing. A placeholder would be enough to let the camera construct, though injecting the real timestamp is cleaner and stays correct if CaSSIS is ever switched to SCLK as that TODO intends. It is suggested to fix this in tgocassis2isis itself rather than relying on the manual editlab step. The ingester already opens and parses the same XML to choose the PSA translation, at tgocassis2isis.cpp lines 71-79, and the em16_tgo_cas exposuretimestamp field is sitting in that same document. A few lines in the PSA-exported path could read it, hex-decode it the way the manual step does, and set SpacecraftClockStartCount on the Instrument group, next to the existing StartTime cleanup. The translation file alone cannot do this because it cannot hex-decode, which is likely why it was left out. That would make the PSA products work for both the ISIS camera and CSM with no manual step, and the editlab line in the recipe above would disappear. I am happy to prepare that ISIS pull request; this thread already has all the information needed. Documenting it on asc-public-docs is worthwhile in the meantime, for anyone ingesting the PSA or ODE exported calibrated framelets. Once the ingester is fixed the note can collapse to a single line pointing at the ISIS version that handles it. Prepared by Claude (Anthropic). |
| } catch (...) { | ||
| throw std::runtime_error( | ||
| "Could not parse the cassis distortion model coefficients."); | ||
| coefficients = std::vector<double>(36, 0.0); |
There was a problem hiding this comment.
what does this do? It's right after the throw.
There was a problem hiding this comment.
Good catch. That line is unreachable dead code: the throw above it unwinds out of the catch, so the zero-fill never runs, and even if it did the local coefficients vector is destroyed during stack unwinding and never returned to any caller, so it would have no effect. The vector is also already default-constructed to a valid empty state at its declaration, so there is no uninitialized-value concern either. I removed it.
The same dead line exists in the older LUNARORBITER, RADTAN, and KPLOSHADOWCAM cases (this case was copied from them). I did not touch those to keep this PR focused, but I am happy to clean them up here or in a separate PR if you prefer.
Done with Claude/AI assistance.
The zero-fill assignment came right after the throw and was unreachable. Drop it. Done with Claude/AI assistance. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The TGO CaSSIS driver was disabled and did not work. This re-enables it and fixes the issues so the generated CSM ISD matches the ISIS camera to a median of about 7e-4 pixel and a maximum of about 0.013 pixel, across 130 framelets of two real stereo pairs (details below).
The fixes:
Validated against ISIS with ASP cam_test on two real stereo pairs at different Mars locations, every framelet: MY34_002014_180_1 (equatorial, 60 framelets: 30 left, 30 right) and MY34_005684_218_1 (southern, 70 framelets: 35 left, 35 right). Across all 130 framelets the ISIS-vs-CSM reprojection error has a median of at most about 7e-4 pixel and a maximum of about 0.013 pixel; the left and right looks are essentially identical (per-look maxima 0.01294, 0.01294 for the first pair and 0.01295, 0.01300 for the second). The maximum is the worst single corner pixel, the round-trip residual of the closed-form distortion, the same behavior as ISIS.
To reproduce one framelet from scratch (the PSA image is public, no login):
which prints, for the ISIS camera versus the CSM camera:
The editlab step is a quirk of the public PSA calibrated product, not part of this change: its exported label does not carry SpacecraftClockStartCount (which spiceinit needs), so it is injected from the XML exposuretimestamp. That is an ISIS ingest detail on the cube side; ALE is unaffected.
Unit tests are in test_cassis_drivers.py. The existing integration test (test_cassis_load) is enabled (it was xfail), and its golden ISD (tests/pytests/data/isds/cassis_isd.json) is updated to the fixed driver's output (summing 1, the cassis distortion, and the half-pixel detector center). The existing test_ephemeris_start_time now mocks utcToEt returning a tuple. New tests: test_sample_summing and test_line_summing for the SummingMode enum, and test_detector_center_sample and test_detector_center_line for the ISIS 0.5-based to CSM 0-based half-pixel conversion.
One minor item: CaSSIS is currently the only instrument that requests the surface light-time correction (it is the only one setting LIGHTTIME_CORRECTION LT+S together with LT_SURFACE_CORRECT), where ISIS samples the target body at the surface-light-time-adjusted time. To match that without changing the shared sensor_position for all sensors, the driver overrides sensor_position for CaSSIS only.
Requires the companion USGSCSM CASSIS distortion PR DOI-USGS/usgscsm#512. Developed with assistance from Claude (Anthropic).
Licensing
This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words: