DOI-USGS / usgscsm

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

Add an initial projective approximation for linescan camera #387

Closed oleg-alexandrov closed 2 years ago

oleg-alexandrov commented 2 years ago

I implemented an initial guess ground-to-image function for the linecan model which is a projective transform, instead of the existing linear transform. The hope is that this makes more sense geometrically when it comes to projections into camera, so it would fit better the function we want. It is also not that nonlinear to be concerned about it doing funny things when out of range.

With the difficult LRO NAC dataset from the repo, the guessed line using this initial guess projective transform differs from the eventual computed value of this line by a mean value 0.69 pixels (sometimes some values can be bigger than 1 or 2 pixels, so this is an average). Before, with the linear approximation, the mean discrepancy was 82.7 pixels.

With a CTX dataset, the mean line error was 160.4 pixels before, and is 0.85 pixels now.

This does not in a dramatic speed improvement though, because the secant method converges so fast even with a bad initial guess. For LRO NAC (when the sample rate in usgscsm_cam_test is 1) the runtime goes from 122 to 108 seconds, which is a 11% speedup, while for CTX it goes from 82 to 70 seconds, which is a 14% speedup (here I used sample rate of 2).

Anyhow, this is a noticeable improvement, verified with two different datasets, so it is not too bad. Likely there's only very limited room for further improvement by fitting a fancier transform.

A couple more notes. I made USGSCSM depend on Eigen, which is not a big deal, as ALE already depends on Eigen and USGSCSM depends on ALE. I tested that the code compiles fine with both internal and external dependencies.

Also, Eigen is a hog and can slow down compilation speed (from 30 to 45 seconds on my machine for the linescan class). For that reason I isolated the Eigen logic to a single .cpp file to avoid this.

oleg-alexandrov commented 2 years ago

I added a unit test, removed the debug logic, wiped the old transform, and all the builds and tests pass.

oleg-alexandrov commented 2 years ago

I think linking to Eigen is not needed because a header file has the implementation. I used the same logic as what is in ALE.

jessemapel commented 2 years ago

Sorry, I don't mean linking as a dynamic library, I mean linking as CMake target vs setting include directories via variables.

oleg-alexandrov commented 2 years ago

The "target_link_libraries" logic should not help. Its doc says: "Specify libraries or flags to use when linking a given target and/or its dependents. ". This should not affect the problem I was seeing with finding headers which was happening before linking.

What you will need, I think is "target_include_directories". Normally even this should not be needed, as "find_package" should already set the header paths, but that was not happening.

Anyhow, I spent a couple of hours yesterday and today to just get things to build. Now things work. If you are feeling adventurous and want to revisit this, one needs to ensure that the build works with both internal and external Eigen (since different logic is used for each), and then see if anything breaks after pushing things.