Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spicinit NumPy warning and file diffs #5224

Closed
lwellerastro opened this issue Jun 12, 2023 · 5 comments · Fixed by #5249
Closed

spicinit NumPy warning and file diffs #5224

lwellerastro opened this issue Jun 12, 2023 · 5 comments · Fixed by #5249
Labels
bug Something isn't working

Comments

@lwellerastro
Copy link
Contributor

ISIS version(s) affected: isis8.0.0-RC2

Description
Spiceinit throws NumPy warning under latest version. The program runs to completion and a Kernels Group is added to the label.
However, a file spiced under this version has different InstrumentPointing/Position Table contents (sizes in particular) when compared to isis8.0.0RC-1. When the images are linked/syned in qview, there is a visible shift between the data, maybe a half pixel or so. Latitude and Longitude values for a pixel differ starting at the 7th decimal location.

How to reproduce
IMG available under my work area Isis3Tests/Spiceinit/NumPyWarning/

lronac2isis from=M110860982LE.IMG to=M110860982LE.cub
spiceinit from=M110860982LE.cub

isis8.0.0-RC2/lib/python3.9/site-packages/scipy/__init__.py:146: UserWarning: A NumPy version >=1.16.5 and <1.23.0 is required for this version of SciPy (detected version 1.24.3
  warnings.warn(f"A NumPy version >={np_minversion} and <{np_maxversion}"

catlab from=M110860982LE.cub to=lab800RC2.pvl

Leading path for env has been stripped.

The same steps were run under isis8.0.0-RC1, then the catlab output was compared. The Kernels Group are identical but there are differences in the following Groups:
InstrumentPointing
InstrumentPosition
BodyRotation
SunPosition

It's possible some of this is expected with changes to ALE, but the Instrument Tables values seem quite different. It's not clear to me what specific changes might be seen between the two versions of ALE.

It's a little difficult sharing a comparison for so many groups, so see files in directory indicated.

Here's a snippet of the Kernels Group and InstrumentPointing Table (diff highlighted in blue), RC2 label on left, RC1 on right:
LabelDiffs_RC2vsRC1

@lwellerastro lwellerastro added the bug Something isn't working label Jun 12, 2023
@acpaquette
Copy link
Collaborator

So comparing ALE 0.8.6 to ALE 0.9.1 is going to seem fairly different but the better comparison is between what ISIS was originally doing and the two versions of ALE.

Here are the differences between ALE 0.8.6 spice data and the the original ISIS spice data as reported on the label (ALE 0.8.6 on the left and the original ISIS data on the right):
Screen Shot 2023-06-13 at 4 54 53 PM
Screen Shot 2023-06-13 at 4 55 11 PM

There is a fair bit different, much of the issues coming from different interpolation methods and sampling of positions or pointing information.

Now we compare ALE 0.9.1 spice data to the original ISIS spice data (Again ALE 0.9.1 on the left and ISIS on the right):

Screen Shot 2023-06-13 at 5 00 48 PM

The largest difference in the label being the solar longitude. So in ALE 0.9.1 we are identical as far as the label is concerned, but that doesn't pain a clear picture as the campt output is still different (ALE 0.9.1 spice on the left and ISIS spice on the right):
Screen Shot 2023-06-13 at 5 11 26 PM

The difference here comes down to ALE doing interpolation when producing instrument/sun position information and only being accurate to ~1e-08 decimal places. See bellow (ALE 0.9.1 spice on the left and ISIS spice on the right):
Screen Shot 2023-06-13 at 5 13 11 PM

I have looked into this and the projected products still retain there same footprint, meaning that most images will project into the exact same lat lon space that they did using the original ISIS spice information. The main difference was images having 5% of there DN values differ between 1e-03 and 1e-08 decimal places.

Just for clarity here is a comparison of the instrument pointing differences between the two (ALE 0.9.1 spice on the left and ISIS spice on the right):
Screen Shot 2023-06-13 at 5 33 14 PM

Most of those differences are floating point error from spice.

With all of that said ALE 0.9.1 is not identical to ISIS but produces results that are much closer than the data ALE 0.8.6 was producing. We can and will look further into the interpolation error generated in the positions. Some solutions have been floated but I would like to explore the solution space a little further.

@acpaquette
Copy link
Collaborator

I have also compared the original ISIS projected cube against both the ALE 0.8.6 and the ALE 0.9.1 projected cubes. Here are the cubediffs from both sets (Left is ALE 0.9.1 compared to the original ISIS data cube, and right is ALE 0.8.6 compared to the original ISIS data cube):

Screen Shot 2023-06-14 at 4 39 17 PM

Basically everything is trending the way we want it to with the new version of ALE. The average difference, standard deviation, and varience are all significantly better. The max difference also isn't even a whole DN value larger, there are no special pixel differences now, and the number of differences is down from ~3% of pixels to ~1.5%.

Another thing to note is the resulting mapping groups compared to the original ISIS data. Here is the ALE 0.8.6 mapping group compared to ISIS (Left ALE 0.8.6 data and right original ISIS data):

Screen Shot 2023-06-14 at 4 46 05 PM

While not huge the difference is notable especially the resolution part.

Here is the mapping group comparison with ALE 0.9.1 data and the original ISIS data (Left ALE 0.9.1 data and right original ISIS data):

Screen Shot 2023-06-14 at 4 46 31 PM

In ALE 0.9.1 there is no longer a resolution disparity and the MinimumLatitude difference is pushed to 1e-11 decimal place from 1e-07 decimal place.

@lwellerastro
Copy link
Contributor Author

Thanks so much for that thorough explanation of the differences @acpaquette! I'll be sure to take this all in to better understand what has changed, but at a glance, things look much improved (when comparing the proper output).

I did not get a chance to do so after I originally posted this, but I have a smaller set of LROC NAC polar test data I will run through isis8.0.0-RC2 from start to finish (including bundle and kernels) and will have closer look at that output. I'll report back to my lroc kernels post #5178 so we can get that closed.

As far as the NumPy warning, I take that's all it is, just a warning with no consequence? I was able to install the latest RC under my own conda environment and there was no warning, so this appears to be an issue for our local astrovm systems only.

@acpaquette
Copy link
Collaborator

@lwellerastro The warning likely has to do with poorly defined dependencies for scipy and hopefully won't cause an issue. Some environments will install with the correct version of numpy/scipy that don't encounter that warning and others will see it, as you mention.

@lwellerastro
Copy link
Contributor Author

@acpaquette, thanks for the explanation.

I just confirmed that the spiced cube under astrovm's 8.0.0-RC2 and my install are identical. So nothing more than warning.

I'm not sure who's responsible (IT?), but the spicy dependencies should probably be addressed on our internal systems at some point because it's pretty unnerving/distracting seeing the warning and may be considered to be an error initially (I thought so anyway). Glad it's just that and nothing more though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants