Add fix for ground to image calculation for LRO NAC cameras

[oleg-alexandrov]

This is a proposed fix for https://github.com/USGS-Astrogeology/usgscsm/issues/338. It makes the ground to image function in the CSM linescan model work correctly for all LRO NAC images I tested (7 of them). It does not change the results for CTX (I tested two datasets) and HiRISE (I tested four of them), and there's no reason for things to change for those based on this fix.

The root of the problem seems to be that the LRO NAC can sometimes, and not always, acquire images in a quirky way. Normally, if the camera travels in orbit from North to South around the Moon, one would expect it to record terrain which gradually also goes from North to South. Yet apparently, sometimes the camera orientation changes in such a way that the scans are actually done South to North (or some other similar flip happens somewhere).

My fix is not ideal, as it doubles the run-time of the ground to image function in cases where things fail with the original implementation (so only for some LRO NAC cases). Should be rather simple to do some logic ahead of time to determine the multiplier value which I now find dynamically, if one knows more about Spice kernels than what I do.

The current fix is enough for me to evaluate the behavior of the CSM models with ASP.

Careful testing should be done though when this is implemented in a more efficient way, because apart from this scan direction quirk, the L and R components of an LRO NAC also need different handling. I will suggest to test, at the minimum, with M1127782730LE, M1127782730RE, M1167355858LE, M1167355858RE. (Maybe examining the LBL files may provide a hint as to how these differ when it comes to scan direction.)

And one should do spiceinit shape = ellipsoid on the cub file before generating the .json file.

[thareUSGS]

@oleg-alexandrov thanks for diagnoses and testing! We have run into this quirk before and I thought we figured it out (perhaps not across enough images though to truly fix it). Awesome. I will let Jesse review the code when he returns next week and then we can test in GXP also.

[barchinal]

@oleg-alexandrov and @thareUSGS, Regarding LROC NAC image collection, I would have thought it was well-known that every 6 months LRO turns 180 degrees (along track) in order to keep the solar array best pointed toward the Sun. There is only one solar array, and it's on the side of the spacecraft, so would otherwise be mostly blocked from the Sun for 6 months if they didn't turn the spacecraft. So when that occurs the LROC NAC images are accumulated in the opposite (top to bottom) direction. The WAC push-frame images are also collected in the opposite direction.

Handling this was built into the LROC model(s) originally done by Annie Howington-Kraus (in SOCET SET and then ISIS), in that somehow she checked on the spacecraft orientation. I don’t know how she implemented it, but one way would be to compare the LRO SPK velocity vector with the (CK derived) orientation of LRO. If something like that is not already included in the CSM model or as part of Oleg’s proposed changes, it should be. Such an implementation might also address the inefficiency problem Oleg describes.

Accounting for this problem might have been easier if on image ingestion LROC images would have a label/flag set with the direction of the spacecraft. I believe I suggested that at the time (2009) but it apparently was never done. That could still be an addition to the ISIS image ingestion process that would help with this.

[jessemapel]

Also, the LROC ALE drivers are already supposed to be accounting for this by computing the flight direction. I wonder if there's an issue with the spiceinit'd cube driver here. I will check on this myself.

[oleg-alexandrov]

I did try myself some kind of heuristic, though perhaps not precise this. It worked for the L image of the L-R pair of a given acquisition (not a stereo pair, the L and R cameras for a given shot). Then it failed for the R image.

The most foolproof way to me is to keep the logic I added but run it just once, in the loader, once everything else is set, by taking a pixel in the middle of the image, shoot it to the ground where it intersects the datum, shoot it back, and see which choice of that "sign" variable returns one back, which is then saved as a member variable going forward.

Anyhow, if you have time to work on this that is great, or if you think the fix refinement I propose now is good enough then I can put it in and have this merged. If, in reasonable time, the USGSCM version is bumped up after a fix, I can incorporate this into an upcoming ASP release, or it can wait for later.

[jessemapel]

That sounds like a good solution. We already have a reference ground point that is computed at the center of the image, so it would be something like:

double achievedPrecision = 1.0;
groundToImage(m_referencePointXyz, 0.001, &achievedPrecision);
if (achievedPrecision > 0.001) {
  m_iTransL[0] *= -1.0;
  m_iTransL[1] *= -1.0;
  m_iTransL[2] *= -1.0;
}

The best place for this would be UsgsAstroLsSensorModel::updateState which does these sort of computations when we first ingest an ISD.

Multiplying the iTransL components also side-steps the need for a new member variable and any changes to the state that would cause.

[oleg-alexandrov]

This turned out to be a bit tricky for a couple of reasons. First, it can happen that the desired precision fails to be achieved either with one choice of m_iTransL or the other. Second, the pixel at which one should do the test matters. It better not be integer as then the test passes easily but may fail later for other pixels.

All the unit tests pass, my own tests with 7 models (LRO NAC, CTX, and HiRISE) pass. I also verified that the correct m_iTransL value gets saved to the json state, and when this state is loaded, things work just as well as with the original ISD.

[oleg-alexandrov]

Indeed, I had realized myself last night the abs was problematic. I put a fix.

When it comes to a test, it will need two isds, one where the sign ends up staying the same and one where it is flipped. I can add a test. I have two such files, of size 145k and 148k. I guess these are small enough to be added to this repo?

[jessemapel]

I went ahead and added a new test ISD which is our basic orbiting LS ISD but with the transL flipped.