NeoGeographyToolkit / StereoPipeline

The NASA Ames Stereo Pipeline is a suite of automated geodesy & stereogrammetry tools designed for processing planetary imagery captured from orbiting and landed robotic explorers on other planets.
Apache License 2.0
478 stars 168 forks source link

LRO NAC pre-processing skips calibration #424

Closed rvwagner closed 5 months ago

rvwagner commented 5 months ago

In the current version (and apparently everything since 30 Dec 2022), the NAC pre-processing script (lronac2mosaic.py) disables all of the calibration options in lronaccal, reducing it to a glorified "cp" command, instead of letting it properly calibrate the image and remove known artifacts.

I'm guessing this was a work-around for a bug in ISIS version 7.1.0, where lronaccal, when calibrating to I/F (the default), would fail to fall back on the built-in kernels if a cube wasn't spiceinit'd, and also wouldn't throw an error message if it was unable to find any SPICE data at all, instead outputting a blank cube with no explanation.  This was fixed in ISIS 7.2.0.

The best fix for ASP would be to move the spiceinit step to run before the lronaccal step, which will ensure that the SPICE data is always available to lronaccal, even if using web spice.  An alternative would be to calibrate to radiance instead, which doesn't need SPICE data at all (add 'RadiometricType=Radiance'). (If you're now on ISIS version 7.2.0 or later, and won't ever use web spice, reverting back to the pre-Dec-2022 code would also work, but moving spiceinit would be nicest for anyone trying to make derivatives of this script.) Regardless, please get rid of the " + ' dark=false nonlinearity=false masked=false radiometric=false' ", which leaves numerous patterned artifacts in the images which likely have a strong negative impact on the resulting DTM quality.

oleg-alexandrov commented 5 months ago

That's a good catch, thanks. I applied these and checked that things work with ISIS 8.0.0 (diff: https://github.com/NeoGeographyToolkit/StereoPipeline/commit/3587bb5634e7be6885fc737f9c86588a1f10b8a0).

You are welcome to fetch the latest version and take it for a spin (https://raw.githubusercontent.com/NeoGeographyToolkit/StereoPipeline/master/src/asp/Tools/lronac2mosaic.py).

rvwagner commented 5 months ago

Wow! That was a phenomenally fast response. I've tested the new script, and it produces much nicer inputs for the stereo matcher. Thanks!

While you're at it, another super-easy risk-free way to improve the default results would be to add spksmithed=true to the spiceinit call, to use higher-quality post-processed spacecraft position data for most of the mission (line 180 becomes cmd = 'spiceinit web=false spksmithed=true from='+ cub). Unfortunately, that's the extent of NAC processing improvements that can be done without either a lot of extra code or downloading large global DTMs. If you're interested in the full details of what can be done, we have a detailed processing guide at https://zenodo.org/records/10055866, but the current script (plus spksmithed=true) should be quite good for most users, and for us perfectionists, there's --resume-at-noproj.

oleg-alexandrov commented 5 months ago

I added spksmithed=true and seems to work well. I added a new option called --spiceinit-options, with default value 'web=false spksmithed=true'. That can be useful for custom processing and to fetch the kernels from the web (though that may be broken or used to be). I updated the manual too (hard browser reload needed).

rvwagner commented 5 months ago

That looks good, and adding in that --spiceinit-options parameter should allow all the important tweaks (mostly allowing web spice, which is very nice during the ~95% of the time that it works, and allowing higher-quality base DTMs than the kinda-iffy one that's built into ISIS).