Closed lwellerastro closed 1 year ago
Here is a screen shot of the campt diffs between spice with ALE (left) vs ISIS (right). Although the differences seen for basic geometric information is not that different, information about the spacecraft, the Sun and some illumination information are a too different. This may highlight how different kernel loading is coming into play. Please see the caminfo output differences via the files in the directory noted in the original post.
@lwellerastro This is good info for ALE and for going foward. We have added a change log to ALE and are doing more to make the progress/changes on that side of things more transparent.
In testing the above with the current version of ALE (0.8.8) the differences are less significant but still present. Something we have not dealt with in ISIS compared to ALE is the solar longitude calculation as it hasn't had a significant affect on processing.
In examining the data more directly it seems like the InstrumentPointing and BodyRotation tables are identical but the InstrumentPosition is off by ~0.000000001 in the X position. The sun position has a more significant difference compared to ISIS with a 0.0000000000001, 0.000000000001, 0.0000000000002 difference in X, Y, and Z velocity. This is likely a result of applying a rotation to those values into the target frame rather keeping them in J2000 so some error is introduced there.
Something else to note is that this issue in ISIS as you point out has been present since ALE was added and you shouldn't even get ALE as a source when you spiceinit a cube a second time.
This is due to there being difference classes of drivers, with the main drivers we want ISIS to use being ISISLabelNaifSpice drivers where we get any label information from ISIS labels and ALE generates fresh spice information. Another class being ISISLabelISISspice drivers. These drivers take ISIS images that already retain spice data and dump them to ISDs in ALE. This was a quicker way to directly compare ISIS to CSM when making new CSM Camera models or making new ALE drivers for use in CSM. Basically a way to compare ISIS to the fourth class of driver in ALE which is PDS3LabelNaifSpice. This driver is able to generate an ISD straight from the PDS3 label, which would be many of the images you can pull directly from the PDS imaging atlas.
This issue has been addressed in ALE, allowing users to only select NaifSpice labels when generating ISDs but it has not yet been released. Once that is released, we can get that into ISIS and you will only get fresh spice data and avoid this issue above entirely.
W.r.t. making the data exact between the two, specifically the ISISLabelISISSpice driver class, I think we could do it but it's a little awkward. Basically we have a formatter that generates ISDs and expects the data to comeback in a certain fashion. If we allow the ISISLabelISISSpice drivers to leave the position information in J2000, but enforce that the ISISLabelNaifSpice information be in body fixed. We run into issues in the formatter because the data is no longer homogeneous. We could enforce that the data that reaches that generic formatter be in J2000 which would better support the ISISLabelISISSpice drivers.
I think a larger discussion out of this could be had at some point. How important is it that the sun information is identical? The pointing information being off is more of an issue. It looks like its off at the 14th sig fig which could more or less be removed by jigsaw/a bundle adjustment.
Thanks for the explanation @acpaquette, though I don't understand much of it. I'm not sure why anything about ISIS spice should need to change in support of ALE - why doesn't ISIS just use ISIS drivers as is all of the time? And why are some missions exclusively picking up ALE like LROC NAC? I'm sorry I don't understand its use within ISIS apps.
Someone like @barchinal or @rkirk are going to have to help address your comments and questions in regard to J2000 vs body fixed (I'm not sure those are interchangeable, but I don't know) and sun information impacts. The latter is very important for photometry and other topics I'm not an expert in. They are there for a reason and shouldn't be considered irrelevant simply because we don't know all the scenarios for which they are used. I think conversations and resolution needs to happen as quickly as possible as several of my projects are impacted by these discoveries.
I think the solution here is making it so ISIS only uses the IsisLabelNaifSpice drivers in ALE for fresh ephemeris data. We can address the solarlongitude differences in another issue.
So I think a good DoD here is:
Enforce ISIS to only use IsisLabelNaifSpice drivers in ALE. If a driver does not exist for the data fall back to the old ISIS routines for attaching spice
This issue has been addressed via isis8.0.0-RC2, though spiceinit throws warnings as indicated in #5224.
A Cassini ISS image ingested under the latest version has source=isis upon the first application of spice and maintains that source/driver when a copy of the file is spiced a second time. The files are also identical as expected.
Thank you!
Resolved in #5209
ISIS version(s) affected:5.0.2 through 8.0.0-RC1
Description
When a Cassini ISS image is spiced for the first time it uses the ISIS Naif driver (pardon me if I'm not describing this properly as it is a new concept for me). If the image is spiced for a second time it uses the ALE Naif driver. This results in geometry differences between the two images. Spicing an image for a second time is not an uncommon practice. In my case, data of Titan underwent a long series of processing to clean up the images and remove the atmosphere that shrouds the surface. This was done several years ago. Since that time (and after creating networks and performing other work) the Cassini Team updated the spacecraft kernel that I needed to pick up. Instead of going back to square one and reprocessing everything from scratch, I made a copy of the clean data and ran spiceinit to pick up the improved kernel. This is no unusual.
Further, as referenced in the Additional context section below, there is at least one, likely two, ALE bugs at play here - the driver may be/is loading kernels in a manner not consistent with ISIS or NAIF.
I suspect other datasets are affected by the changing driver problem (isis switching to ale after a second run of spiceinit), and it seems all instruments that either exclusively or accidentally use the ale driver are affected by kernels possibly not loading in the proper order. Both of these problems are affecting multiple IAA projects and those of high priority.
How to reproduce
See data under my work area directory Isis3Tests/Spiceinit/ISISvsALE/
I'll demo here with references to isis5.0.2, but the results through all version of isis up to the present.
The campt and caminfo output show the differences, some much larger than others. These files should be identical.
Possible Solution
Besides obviously correcting these problems, is there any way for a user to specify which driver (isis vs ale) they would like to use? Changes to ALE may and have directly impacted ISIS in a negative and until these are resolved, is there any way a user can avoid ale without going back to pre-ale versions of isis (which isn't possible for at least LROC, see post #5201 )
Documentation for ALE needs to be improved. There is no information in the ALE CHANGELOG describing any of the changes to that repository for anyone to refer to, and no discussion in the ISIS CHANGELOG about ALE updates/impacts since it was first introduced and described in 2020. Any changes to spice, even in a positive and improved way must be documented and made available to users.
Additional context
Much of what is being shared here from the user perspective is based on comments from post #5178.
https://github.com/DOI-USGS/ISIS3/issues/5178#issuecomment-1500736068 I believe this is what is being seen in the simple Cassini ISS test described above (source=isis switching to ale after a second spiceinit), but the description shared in this comment is difficult for me to understand. It's possible the linked comments have no correlation to what is happening with the Cassini example and it could be a different issue.
https://github.com/DOI-USGS/ISIS3/issues/5178#issuecomment-1539190295 The direct impact of this revelation are two-fold - a) images using the ALE driver may never have kernels loaded in the proper order picking up incorrect spice, and b) an attempt to override other kernels via the ISIS spiceinit EXTRA parameter may not occur because those kernels must be loaded last to take precedence but might be sorted in a such a way by ALE that they may be overwritten by the contents of other kernels that were loaded after them, also resulting in unexpected/incorrect results.
I believe kernels generated by ckwriter and spkwriter for images using the ALE driver would not be accurately reflected for images accessing those kernels but via the isis driver (in the case where that driver is picked up by spiceinit the first time) like in my Cassini project example.