DOI-USGS / usgscsm

This repository stores USGS Community Sensor Model (CSM) camera models
Other
26 stars 33 forks source link

Use image center when computing flying height #485

Closed oleg-alexandrov closed 3 months ago

oleg-alexandrov commented 3 months ago

When a CSM linescan model is loaded, some auxiliary info is computed, including the GSD, reference point, and flying height.

All but the last one use the image center (half image dimensions) for the computations. But the flying height uses time 0. Time 0 is not well-defined for a linescan sensor. One should use either the starting time as set in the metadata, or the time at starting line, or some time that is relevant to the given sensor.

I ran into this when creating custom linescan models. The resulting flying height then ends up being quite off, depending on what the starting time is.

The proposed fix uses the image center time to find the flying height, for consistency with GSD and reference point calculations.

The effect of this fix in practice is very small, unless one does unusual things.

Looks like all tests still pass after the fix.

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:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

acpaquette commented 3 months ago

@oleg-alexandrov This looks fine, my understanding with USGSCSM is that time "0" is the middle of the line scan exposure since the times get offset by the center time. So your beginning times should be negative approaching 0 at the middle, then increasing and remaining positive till the end of the image. It also seems weird that this change didn't effect the tests at all. Is there some dataset or situation where this change improves/accurately calculates the flying height?

oleg-alexandrov commented 3 months ago

After looking through the code, I can confirm that the starting time that gets saved to the model state file, which is m_t0Ephem, is indeed such that time 0 is the middle of the line. So, that is not the actual clock time, but got shifted internally. So, the code is consistent.

The problem shows up if one starts with a model state file, which is perfectly self-consistent, and modifies m_t0Ephem, m_t0Quat, and m_intTimeStartTimes to a new value, and forces computation of GSD, reference point, and flying height.

Then, the first two work fine, but the latter does not, because time 0 is now out of sync with starting time.

This also explains why no tests are failing, as the problem shows up if one creates a model state file directly, rather than starting with provided raw clock, etc.

I need to be able to create such model state files because I use CSM with synthetic data, when it would be too much work to imitate all the process starting with querying spice, J2000, etc., and model state files are supposed anyway to be able to hold a valid model.

Here's how to reproduce that the current logic has this issue:

This will now result in a bad flying height. Before, it was 315 km, now it will be 3415 km.

acpaquette commented 3 months ago

Sorry, just trying to understand what we are doing here.

It worries me that we are manually editing times that apply to ephemeris data. By adjusting the start time are we assuming that the interpolation will correctly apply the motion of the spacecraft to whatever time we set?

I am trying to get a scope for the above justification. It seems like we would need to recompute ephemeris times anyway if we are going to go back in time 990 seconds.

If you avoiding changing the starting times, then the flying height and GSD are recomputed correctly by USGSCSM

oleg-alexandrov commented 3 months ago

What you say is correct.

The question is if there exists a precise standard for model state files that spells out all the conventions. Is time 0 required to be at the center of the line? Or can one take a model state file, and shift all times forward by the same amount, and still be guaranteed to get the same result?

For now I think there is no standard either way, and what exists is for convenience of implementation. Then, we could as well apply the change I am suggesting, which explicitly sets the center of the image as the locations relative to which various calculations take place.

acpaquette commented 3 months ago

Cool, just wanted to make sure that this is a bit of an abuse of the model state but glad it exposed a potential bug!

oleg-alexandrov commented 3 months ago

Thanks, Adam.