From e331f5090209643e0730a5803f93f36c4cd093aa Mon Sep 17 00:00:00 2001 From: Oleg Alexandrov Date: Tue, 3 Nov 2026 14:02:59 -0800 Subject: [PATCH 1/5] Fail ISD generation on null interior-orientation fields SpiceQL can return null for required interior-orientation fields such as focal2pixel_lines, focal_length, or the detector center, for example when a web SpiceQL query fails. Driver.to_dict catches the error and returns None, so the ISD was written with a null field and isd_generate still reported success. That camera then crashes or behaves undefined downstream in usgscsm, such as an empty focal2pixel transform or a zero focal length. The formatter now checks these required fields and raises a clear error instead. Made with Claude AI assistance. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + ale/formatters/formatter.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8ae30484..2cd96ab47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ release. - ISIS-SPICE and NAIF-SPICE drivers: subtract 0.5 from the detector center (ISIS 0.5-based to CSM 0-based). - NAIF-SPICE driver: `focal_length` no longer raises `AttributeError` when `INS_FOCAL_LENGTH` is defined. - NAIF-SPICE driver: `ephemeris_start_time` uses the scalar from `utcToEt` (was a tuple). [#739](https://github.com/DOI-USGS/ale/pull/739) +- `isd_generate` now raises instead of writing an ISD with null interior-orientation fields (`focal_length`, `detector_center`, `focal2pixel_lines`, `focal2pixel_samples`). SpiceQL can return these as null (for example when a web query fails), and accepting them silently produces a camera that crashes or behaves undefined downstream in usgscsm, such as an empty focal2pixel transform or a zero focal length. [#750](https://github.com/DOI-USGS/ale/pull/750) ### Changed - `isd_generate`: default `--reduction` to `linear` (sampling every 10th line for line-scan images with >= 1000 lines). This avoids creating oversized ISDs (tens of thousands of pose samples) for long line-scan sensors such as LRO NAC, MRO CTX, and HRSC. Pass `--reduction none` to disable reduction and restore per-line sampling. [#744](https://github.com/DOI-USGS/ale/pull/744) diff --git a/ale/formatters/formatter.py b/ale/formatters/formatter.py index 38070c8fb..cad6c173c 100644 --- a/ale/formatters/formatter.py +++ b/ale/formatters/formatter.py @@ -170,6 +170,14 @@ def to_isd(driver): isd['starting_detector_line'] = driver_data["detector_start_line"] isd['starting_detector_sample'] = driver_data["detector_start_sample"] + # these interior-orientation fields are required for any CSM model + required = ['focal_length', 'detector_center_line', 'detector_center_sample', + 'focal2pixel_lines', 'focal2pixel_samples'] + null_fields = [k for k in required if driver_data.get(k) is None] + if null_fields: + raise RuntimeError("Could not generate a valid ISD. Required field(s) " + "are null: " + ", ".join(null_fields)) + j2000_rotation = frame_chain.compute_rotation(target_frame, J2000) instrument_position = {} From 87e24ffaf1f83a39baf8303c447c4a0f5f607488 Mon Sep 17 00:00:00 2001 From: Oleg Alexandrov Date: Tue, 3 Nov 2026 14:03:38 -0800 Subject: [PATCH 2/5] CHANGELOG: point the entry at PR 754 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cd96ab47..063219974 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ release. - ISIS-SPICE and NAIF-SPICE drivers: subtract 0.5 from the detector center (ISIS 0.5-based to CSM 0-based). - NAIF-SPICE driver: `focal_length` no longer raises `AttributeError` when `INS_FOCAL_LENGTH` is defined. - NAIF-SPICE driver: `ephemeris_start_time` uses the scalar from `utcToEt` (was a tuple). [#739](https://github.com/DOI-USGS/ale/pull/739) -- `isd_generate` now raises instead of writing an ISD with null interior-orientation fields (`focal_length`, `detector_center`, `focal2pixel_lines`, `focal2pixel_samples`). SpiceQL can return these as null (for example when a web query fails), and accepting them silently produces a camera that crashes or behaves undefined downstream in usgscsm, such as an empty focal2pixel transform or a zero focal length. [#750](https://github.com/DOI-USGS/ale/pull/750) +- `isd_generate` now raises instead of writing an ISD with null interior-orientation fields (`focal_length`, `detector_center`, `focal2pixel_lines`, `focal2pixel_samples`). SpiceQL can return these as null (for example when a web query fails), and accepting them silently produces a camera that crashes or behaves undefined downstream in usgscsm, such as an empty focal2pixel transform or a zero focal length. [#754](https://github.com/DOI-USGS/ale/pull/754) ### Changed - `isd_generate`: default `--reduction` to `linear` (sampling every 10th line for line-scan images with >= 1000 lines). This avoids creating oversized ISDs (tens of thousands of pose samples) for long line-scan sensors such as LRO NAC, MRO CTX, and HRSC. Pass `--reduction none` to disable reduction and restore per-line sampling. [#744](https://github.com/DOI-USGS/ale/pull/744) From 451b81e01f310797b1534b134be70d80845715c2 Mon Sep 17 00:00:00 2001 From: Oleg Alexandrov Date: Tue, 3 Nov 2026 14:45:56 -0800 Subject: [PATCH 3/5] Propagate the invalid-ISD error and check positions and orientations Addresses review feedback on the null-field check. The driver loader wrapped to_isd in a broad except that retried other drivers, so a raise in the formatter was swallowed and the user got a misleading "No viable Driver for Label" instead of the real cause. Introduce an InvalidISDError that the loader re-raises once a driver has already matched the label, so the clear message reaches the user. Also check the other required fields, the sensor positions, sun position, and frame chain (orientations), not only the interior orientation. Made with Claude AI assistance. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- ale/base/__init__.py | 8 ++++++++ ale/drivers/__init__.py | 6 +++++- ale/formatters/formatter.py | 24 ++++++++++++++++++------ 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 063219974..6ca8bf1e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ release. - ISIS-SPICE and NAIF-SPICE drivers: subtract 0.5 from the detector center (ISIS 0.5-based to CSM 0-based). - NAIF-SPICE driver: `focal_length` no longer raises `AttributeError` when `INS_FOCAL_LENGTH` is defined. - NAIF-SPICE driver: `ephemeris_start_time` uses the scalar from `utcToEt` (was a tuple). [#739](https://github.com/DOI-USGS/ale/pull/739) -- `isd_generate` now raises instead of writing an ISD with null interior-orientation fields (`focal_length`, `detector_center`, `focal2pixel_lines`, `focal2pixel_samples`). SpiceQL can return these as null (for example when a web query fails), and accepting them silently produces a camera that crashes or behaves undefined downstream in usgscsm, such as an empty focal2pixel transform or a zero focal length. [#754](https://github.com/DOI-USGS/ale/pull/754) +- `isd_generate` now raises a clear `InvalidISDError` instead of writing an ISD with null required fields (positions, orientations, focal length, detector center, focal2pixel transforms). `Driver.to_dict` returns null for any property whose SPICE/SpiceQL query failed (for example a failed web query), and accepting these silently produced a camera that crashed or behaved undefined downstream in usgscsm, such as an empty focal2pixel transform or a zero focal length. The driver loader no longer swallows this as a wrong-driver error, so the real message reaches the user. [#754](https://github.com/DOI-USGS/ale/pull/754) ### Changed - `isd_generate`: default `--reduction` to `linear` (sampling every 10th line for line-scan images with >= 1000 lines). This avoids creating oversized ISDs (tens of thousands of pose samples) for long line-scan sensors such as LRO NAC, MRO CTX, and HRSC. Pass `--reduction none` to disable reduction and restore per-line sampling. [#744](https://github.com/DOI-USGS/ale/pull/744) diff --git a/ale/base/__init__.py b/ale/base/__init__.py index a03106b2f..fe4719a15 100644 --- a/ale/base/__init__.py +++ b/ale/base/__init__.py @@ -12,6 +12,14 @@ class WrongLabelTypeException(Exception): """ pass +class InvalidISDError(Exception): + """ + Exception raised when a driver matches the label but the generated ISD is + invalid, for example a required field is null. Unlike a wrong-driver + exception, this is not retried against other drivers. + """ + pass + spiceql_mission_map = { "APOLLO_METRIC" : "apollo15_metric", diff --git a/ale/drivers/__init__.py b/ale/drivers/__init__.py index b5cc32f68..9d1cb0cf3 100644 --- a/ale/drivers/__init__.py +++ b/ale/drivers/__init__.py @@ -11,7 +11,7 @@ import os import os.path import traceback -from ale.base import WrongInstrumentException, WrongLabelTypeException +from ale.base import WrongInstrumentException, WrongLabelTypeException, InvalidISDError import logging from ale.formatters.formatter import to_isd @@ -220,6 +220,10 @@ def load_from_label(label, props={}, formatter='ale', verbose=False, only_isis_s except WrongInstrumentException as e: if verbose: logger.info(f"Wrong instrument id for driver {driver.__class__.__name__}: {e}. Skipping driver.") + except InvalidISDError: + # the driver matched the label but produced an invalid ISD, so do + # not retry other drivers, surface the real error to the user + raise except Exception as e: if verbose: traceback.print_exc() diff --git a/ale/formatters/formatter.py b/ale/formatters/formatter.py index cad6c173c..8dd4d84f7 100644 --- a/ale/formatters/formatter.py +++ b/ale/formatters/formatter.py @@ -5,6 +5,7 @@ import logging from ale.base.type_sensor import LineScanner, Framer, Radar, PushFrame +from ale.base import InvalidISDError from ale import logger def to_isd(driver): @@ -23,6 +24,17 @@ def to_isd(driver): """ driver_data = driver.to_dict() + + # driver.to_dict() returns None for any property whose computation raised + # (for example a failed SPICE/SpiceQL query). These are required for any + # CSM model, and accepting them as null writes a broken ISD that fails only + # later, opaquely, in usgscsm. Fail here instead, naming the null fields. + required = ['frame_chain', 'sensor_position', 'sun_position'] + null_fields = [k for k in required if driver_data.get(k) is None] + if null_fields: + raise InvalidISDError("Could not generate a valid ISD. Required field(s) " + "are null: " + ", ".join(null_fields)) + if logger.isEnabledFor(logging.DEBUG): for k, v in driver_data.items(): if isinstance(v, list): @@ -171,12 +183,12 @@ def to_isd(driver): isd['starting_detector_sample'] = driver_data["detector_start_sample"] # these interior-orientation fields are required for any CSM model - required = ['focal_length', 'detector_center_line', 'detector_center_sample', - 'focal2pixel_lines', 'focal2pixel_samples'] - null_fields = [k for k in required if driver_data.get(k) is None] - if null_fields: - raise RuntimeError("Could not generate a valid ISD. Required field(s) " - "are null: " + ", ".join(null_fields)) + io_required = ['focal_length', 'detector_center_line', 'detector_center_sample', + 'focal2pixel_lines', 'focal2pixel_samples'] + io_null = [k for k in io_required if driver_data.get(k) is None] + if io_null: + raise InvalidISDError("Could not generate a valid ISD. Required field(s) " + "are null: " + ", ".join(io_null)) j2000_rotation = frame_chain.compute_rotation(target_frame, J2000) From 413dc68d8b0155ceb366f8d516258b1642ec9735 Mon Sep 17 00:00:00 2001 From: Oleg Alexandrov Date: Tue, 3 Nov 2026 14:49:12 -0800 Subject: [PATCH 4/5] CHANGELOG: trim the entry to one user-facing line Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ca8bf1e0..2a57c35a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ release. - ISIS-SPICE and NAIF-SPICE drivers: subtract 0.5 from the detector center (ISIS 0.5-based to CSM 0-based). - NAIF-SPICE driver: `focal_length` no longer raises `AttributeError` when `INS_FOCAL_LENGTH` is defined. - NAIF-SPICE driver: `ephemeris_start_time` uses the scalar from `utcToEt` (was a tuple). [#739](https://github.com/DOI-USGS/ale/pull/739) -- `isd_generate` now raises a clear `InvalidISDError` instead of writing an ISD with null required fields (positions, orientations, focal length, detector center, focal2pixel transforms). `Driver.to_dict` returns null for any property whose SPICE/SpiceQL query failed (for example a failed web query), and accepting these silently produced a camera that crashed or behaved undefined downstream in usgscsm, such as an empty focal2pixel transform or a zero focal length. The driver loader no longer swallows this as a wrong-driver error, so the real message reaches the user. [#754](https://github.com/DOI-USGS/ale/pull/754) +- `isd_generate` now fails with an error instead of writing an ISD with null required fields. [#754](https://github.com/DOI-USGS/ale/pull/754) ### Changed - `isd_generate`: default `--reduction` to `linear` (sampling every 10th line for line-scan images with >= 1000 lines). This avoids creating oversized ISDs (tens of thousands of pose samples) for long line-scan sensors such as LRO NAC, MRO CTX, and HRSC. Pass `--reduction none` to disable reduction and restore per-line sampling. [#744](https://github.com/DOI-USGS/ale/pull/744) From 24d3afaf84b95c7cd0ae63bd2fe97bd9d8aa64d0 Mon Sep 17 00:00:00 2001 From: Oleg Alexandrov Date: Tue, 3 Nov 2026 15:03:17 -0800 Subject: [PATCH 5/5] Do not break driver fallback on the invalid-ISD check The first attempt checked positions and orientations right after to_dict and re-raised InvalidISDError immediately. That broke the driver-selection loop, which tries several drivers for a label and relies on the wrong ones failing so it can fall through to the right one. Those wrong drivers legitimately return null for the frame chain and positions, so the eager check aborted before the correct driver was reached (CTX, MEX SRC load tests failed). Keep only the interior-orientation check, which runs late in to_isd after the frame chain and positions are already used, so a driver reaching it is the real one. The loader now remembers an InvalidISDError and keeps trying other drivers, raising it only if none succeed, so fallback still works and the clear message still reaches the user when the matched driver is the one that failed. Made with Claude AI assistance. Co-Authored-By: Claude Opus 4.8 (1M context) --- ale/drivers/__init__.py | 14 ++++++++++---- ale/formatters/formatter.py | 11 ----------- 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/ale/drivers/__init__.py b/ale/drivers/__init__.py index 9d1cb0cf3..8e440e5f8 100644 --- a/ale/drivers/__init__.py +++ b/ale/drivers/__init__.py @@ -197,6 +197,7 @@ def load_from_label(label, props={}, formatter='ale', verbose=False, only_isis_s parsed_label = pre_parse_label(label) + invalid_isd_error = None for driver in drivers: if verbose: logger.info(f'Trying {driver.__name__}') @@ -220,10 +221,13 @@ def load_from_label(label, props={}, formatter='ale', verbose=False, only_isis_s except WrongInstrumentException as e: if verbose: logger.info(f"Wrong instrument id for driver {driver.__class__.__name__}: {e}. Skipping driver.") - except InvalidISDError: - # the driver matched the label but produced an invalid ISD, so do - # not retry other drivers, surface the real error to the user - raise + except InvalidISDError as e: + # the driver produced an invalid ISD (a required field was null). + # Keep trying other drivers, but remember this so that if none + # succeed we surface this real error instead of a generic one. + invalid_isd_error = e + if verbose: + logger.info(f"Invalid ISD from driver {driver.__class__.__name__}: {e}. Skipping driver.") except Exception as e: if verbose: traceback.print_exc() @@ -231,6 +235,8 @@ def load_from_label(label, props={}, formatter='ale', verbose=False, only_isis_s logger.info(f'End {driver}\n') if verbose: logger.setLevel(logger_level) + if invalid_isd_error is not None: + raise invalid_isd_error raise Exception('No viable Driver for Label.') diff --git a/ale/formatters/formatter.py b/ale/formatters/formatter.py index 8dd4d84f7..c274c9846 100644 --- a/ale/formatters/formatter.py +++ b/ale/formatters/formatter.py @@ -24,17 +24,6 @@ def to_isd(driver): """ driver_data = driver.to_dict() - - # driver.to_dict() returns None for any property whose computation raised - # (for example a failed SPICE/SpiceQL query). These are required for any - # CSM model, and accepting them as null writes a broken ISD that fails only - # later, opaquely, in usgscsm. Fail here instead, naming the null fields. - required = ['frame_chain', 'sensor_position', 'sun_position'] - null_fields = [k for k in required if driver_data.get(k) is None] - if null_fields: - raise InvalidISDError("Could not generate a valid ISD. Required field(s) " - "are null: " + ", ".join(null_fields)) - if logger.isEnabledFor(logging.DEBUG): for k, v in driver_data.items(): if isinstance(v, list):