Fix for very large number of samples#677
Merged
acpaquette merged 9 commits intoDOI-USGS:mainfrom Mar 23, 2026
Merged
Conversation
Kelvinrr
reviewed
Nov 14, 2025
ale/base/type_sensor.py
Outdated
Comment on lines
+100
to
+104
| # A pose sample very 10 lines is more than enough. Otherwise | ||
| # the .json files become very large and very slow to load. | ||
| numSamples = max(self.image_lines/10, 100) | ||
| numSamples = min(numSamples, self.image_lines) | ||
| self._ephemeris_time = np.linspace(self.ephemeris_start_time, self.ephemeris_stop_time, numSamples + 1) |
Collaborator
There was a problem hiding this comment.
This should be configurable in some way. We have used the props parameter on init to pass kwargs into the driver objects. We could check if it got mixed in; otherwise, use a default.
Sample at max(100, image_lines // 10) instead of one per line. Large linescan images (e.g., Chandrayaan-2 TMC with 177k lines) produced huge ISDs that were slow to generate and load. Tested with CTX (Mars, 11k lines) and Chandrayaan-2 TMC (Moon, 177k lines) via NaifSpice. Results: - CTX: 0.000034 px max error, ISD 2.5 MB -> 0.3 MB, 3.3x faster load - Ch2 TMC: 0.02 px max error, ISD 65 MB -> 6.5 MB, 8.7x faster load The sample count can be overridden via props['num_ephem_samples']. Removes the Chandrayaan-specific ephemeris_time override, now handled by the general fix in the base class.
oleg-alexandrov
force-pushed
the
ephem_fix
branch
from
027d1b4 to
40cd287
Compare
Eigen 5.0.1 on conda-forge is not accepted by find_package(Eigen3 3.3) due to major version mismatch. The API is backward-compatible, so drop the version requirement.
Update 20 reference ISD files to match the new ephemeris_time sampling in type_sensor.py (max(25, image_lines // 10) instead of image_lines + 1). Sample counts reduced ~4-10x depending on instrument. Position and orientation values remain identical at shared times since SPICE polynomial evaluation is exact regardless of query density.
Only apply 1:10 reduction when the result is at least 25 samples
(USGSCSM Lagrange interpolation minimum). For images under ~250 lines,
keep original one-per-line sampling to avoid unnecessary changes.
Formula: reduced = image_lines // 10 + 1
num_samples = reduced if reduced >= 25 else image_lines + 1
Update 13 reference ISD files for large-image instruments where the 1:10 ephemeris reduction applies. Small-image ISDs are unchanged. Fix pre-existing test_transformation floating-point comparison by using assert_allclose with atol=1e-15 instead of assert_equal.
These ISDs were incorrectly regenerated with the old floor-of-100 formula. With the corrected formula, small images keep one-per-line sampling, so their golden ISDs should match upstream main.
Only reduce sampling for images with 1000+ lines (where lines//10+1 >= 100). Smaller images keep one-per-line to preserve existing behavior. Only 4 ISDs change: mexhrsc, mexhrsc_isis, mgsmocna, hirise.
Collaborator
Author
|
I updated the fix to this to ensure ISDs with less than 1000 lines are not modified as that was changing too many results. The custom fix for Chandraayaan got removed as it inherits it from the parent class. Also: fixed a recent Eigen compile error. Added the ability to pass in a custom number of samples via the props field. Here is how results change when doing image to ground with old ISD and then back ground to image with new ISD, and back with the ASP cam_test. Here are max pixel abs difference result.
Note that the resampling also works for the MGS MOC test. Number of samples is reduced properly. But this seems to be a funny test where rays do not point to ground, so the cam_test approach fails to validate. Manual inpsection looks ok. |
acpaquette
reviewed
Mar 23, 2026
Collaborator
acpaquette
left a comment
acpaquette
left a comment
There was a problem hiding this comment.
One change to address and a topic to discuss
Replace num_ephem_samples prop with reduction/ephem_sample_rate props per reviewer request. reduction="linear" (default) subsamples every Nth line; reduction=None keeps one-per-line. Remove OHRC ephemeris_time override, now handled by base class.
acpaquette
reviewed
Mar 23, 2026
Collaborator
acpaquette
left a comment
acpaquette
left a comment
There was a problem hiding this comment.
Just needs a Changelog entry and this should be good to go!
acpaquette
approved these changes
Mar 23, 2026
Collaborator
Author
|
Added changelog. Tests pass. |
jrcain-usgs
pushed a commit
to jrcain-usgs/ale
that referenced
this pull request
Mar 25, 2026
* Reduce ephemeris_time samples for linescan sensors
Sample at max(100, image_lines // 10) instead of one per line.
Large linescan images (e.g., Chandrayaan-2 TMC with 177k lines)
produced huge ISDs that were slow to generate and load.
Tested with CTX (Mars, 11k lines) and Chandrayaan-2 TMC (Moon, 177k
lines) via NaifSpice. Results:
- CTX: 0.000034 px max error, ISD 2.5 MB -> 0.3 MB, 3.3x faster load
- Ch2 TMC: 0.02 px max error, ISD 65 MB -> 6.5 MB, 8.7x faster load
The sample count can be overridden via props['num_ephem_samples'].
Removes the Chandrayaan-specific ephemeris_time override, now handled
by the general fix in the base class.
* Drop Eigen3 version constraint to support Eigen 5.x
Eigen 5.0.1 on conda-forge is not accepted by find_package(Eigen3 3.3)
due to major version mismatch. The API is backward-compatible, so drop
the version requirement.
* Regenerate golden ISDs for reduced ephemeris sampling
Update 20 reference ISD files to match the new ephemeris_time sampling
in type_sensor.py (max(25, image_lines // 10) instead of image_lines + 1).
Sample counts reduced ~4-10x depending on instrument. Position and
orientation values remain identical at shared times since SPICE
polynomial evaluation is exact regardless of query density.
* Fix ephemeris sampling formula for small images
Only apply 1:10 reduction when the result is at least 25 samples
(USGSCSM Lagrange interpolation minimum). For images under ~250 lines,
keep original one-per-line sampling to avoid unnecessary changes.
Formula: reduced = image_lines // 10 + 1
num_samples = reduced if reduced >= 25 else image_lines + 1
* Regenerate golden ISDs and fix rotation test tolerance
Update 13 reference ISD files for large-image instruments where the
1:10 ephemeris reduction applies. Small-image ISDs are unchanged.
Fix pre-existing test_transformation floating-point comparison by
using assert_allclose with atol=1e-15 instead of assert_equal.
* Restore original ISDs for small-image instruments
These ISDs were incorrectly regenerated with the old floor-of-100
formula. With the corrected formula, small images keep one-per-line
sampling, so their golden ISDs should match upstream main.
* Raise ephemeris sampling floor to 100 and regenerate ISDs
Only reduce sampling for images with 1000+ lines (where lines//10+1
>= 100). Smaller images keep one-per-line to preserve existing
behavior. Only 4 ISDs change: mexhrsc, mexhrsc_isis, mgsmocna, hirise.
* Address review: rename props API, remove OHRC override
Replace num_ephem_samples prop with reduction/ephem_sample_rate props
per reviewer request. reduction="linear" (default) subsamples every Nth
line; reduction=None keeps one-per-line. Remove OHRC ephemeris_time
override, now handled by base class.
* Add changelog entry for ephemeris subsampling
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a proposed fix for the fact that ALE creates outrageously large linescan models, with one pose sample per line, which makes mapprojection and bundle adjustment almost unusable, as a huge amount of time is spent simply loading the cameras.
The fix is to produce one pose sample per 10 lines. This should be more than enough, given how little the camera poses change in orbit.
There is no reason to think the current approach of one sample per line is more accurate. All of these interpolate into the true number of samples recorded in spice in either case. Then, in the usgscsm linescan model, there's additional Lagrange interpolation.
I tested this very carefully for LRO NAC, and I will be using this for more than 1000 images for a Moon map project. Projecting from the camera to the ground and back brings one to within 0.061 pixels of starting pixel.
Maybe a different fix is desired, as proposed in #655.
But I think there should be a sense of urgency for this as the problem is rather notable and the fix is rather simple.
Here are some good LRO NAC images to test with: M1285980578RE, M1104890331LE, M1104883188RE.
The ASP cam_test program can test old vs new implementation.
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: