Update gllssical to work without local spice kernels #4306

krlberry · 2021-02-22T20:31:31Z

Description

Updates gllssical to use getClockTime() function, which will use the already-saved ET for the Start Time of the image if it has already been saved, or fall back to furnishing the sclk kernel if this fails.

Related Issue

#4303

Motivation and Context

See #4303

How Has This Been Tested?

Tested by hand and using existing tests. Existing test data needed to be re-spiceinited to work with updates, and the results are slightly different with the new kernels and updates. Anyone have thoughts about whether these changes are reasonable or not?

Difference in calibration results for default gllssical tests:

3439R.cal.pvl:

1213r.cal.pvl:

There are also DN changes on the calibrated cubes:

3439R.cal.cub:

Group = Results
  Compare                   = Different
  Sample                    = 2
  Line                      = 1
  Band                      = 1
  AverageDifference         = 1.59002414671235e-08
  StandardDeviation         = 2.29613941470193e-08
  Variance                  = 5.27225621174773e-16
  MinimumDifference         = 9.09494701772928e-13
  MaximumDifference         = 5.96046447753906e-08
  MaxDifferenceSample       = 161
  MaxDifferenceLine         = 2
  MaxDifferenceBand         = 1
  ValidPixelDifferences     = 63568
  SpecialPixelDifferences   = 0
  SigFigAccuracy            = 6
  SigFigMaxDifferenceSample = 711
  SigFigMaxDifferenceLine   = 10
  SigFigMaxDifferenceBand   = 1
End_Group

1213r.cal.cub:

Group = Results
  Compare                   = Different
  Sample                    = 3
  Line                      = 1
  Band                      = 1
  AverageDifference         = 5.00284445778649e-08
  StandardDeviation         = 1.40651587506991e-08
  Variance                  = 1.97828690682368e-16
  MinimumDifference         = 1.49011611938477e-08
  MaximumDifference         = 1.19209289550781e-07
  MaxDifferenceSample       = 3
  MaxDifferenceLine         = 1
  MaxDifferenceBand         = 1
  ValidPixelDifferences     = 161138
  SpecialPixelDifferences   = 0
  SigFigAccuracy            = 7
  SigFigMaxDifferenceSample = 3
  SigFigMaxDifferenceLine   = 1
  SigFigMaxDifferenceBand   = 1
End_Group

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Documentation change (update to the documentation; no code change)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have read and agree to abide by the Code of Conduct
I have read the CONTRIBUTING document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.
I have added myself to the .zenodo.json document.
I have added any user impacting changes to the CHANGELOG.md document.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

isis/src/galileo/objs/SsiCamera/SsiCamera.cpp

scsides

So this one was basically (Spice object) using the camera already thus requiring spiceinit. May want to put this one on the list for just removing the try/catch and getting a camera off the cube. If we can't get a Camera, there is no chance we can get a Spice since they both require that spiceinit ran. Suggest simplifying the code, but what is there is correct.

scsides · 2021-02-22T21:47:22Z

If my calculations are correct this is only a 12km difference. Nearly nothing in the grand scheme of sun light getting to the target.
Edited: The estimate was off. I didn't see the / 5.xx

scsides · 2021-02-24T01:08:06Z

isis/src/galileo/apps/gllssical/main.cpp

+    catch (IException &e) {
+      Isis::FileName sclk(label->findGroup("Kernels", Pvl::Traverse)["SpacecraftClock"][0]); 
+      QString sclkName(sclk.expanded()); 
+      furnsh_c(sclkName.toLatin1().data());


This needs a naif check around it with a catch. If the furnish fails everything after that will be bad.

krlberry · 2021-02-24T16:24:58Z

isis/src/galileo/apps/gllssical/main.cpp

+      rsun = cam->sunToBodyDist() / 1.49597870691E8 / 5.2;
+    } 
+    catch (IException &e) {
+      // try original fallback for previously spiceinited data 


Previously/old/prior to this update spiceinited data doesn't have all the information it needs to go through the camera.

scsides · 2021-02-24T22:52:04Z

isis/src/galileo/apps/gllssical/main.cpp

-     *      -------- * --- (D/5.2)**2
-     *         A1       Ko
-     */
+    try {


Not sure the 3 lines above are used except in the catch

Good catch, thanks! I'll move it into the catch.

scsides · 2021-02-24T23:05:40Z

isis/src/galileo/apps/gllssical/tsts/default/Makefile

 	$(APPNAME) FROM=$(INPUT)/3439R.cub TO=$(OUTPUT)/3439R.cal.cub > /dev/null;
 	catlab FROM=$(OUTPUT)/3439R.cal.cub TO=$(OUTPUT)/3439R.cal.pvl > /dev/null;
 	$(APPNAME) FROM=$(INPUT)/1213r.cub TO=$(OUTPUT)/1213r.cal.cub > /dev/null;
 	catlab FROM=$(OUTPUT)/1213r.cal.cub TO=$(OUTPUT)/1213r.cal.pvl > /dev/null;
+
+	# Test newly re-spiceinited data
+	$(APPNAME) FROM=$(INPUT)/3439R.respiceinit.cub TO=$(OUTPUT)/3439R.respiceinit.cub > /dev/null;


Was the original cube spiceinit'ed, but you couldn't get a camera off it?

Yes, exactly. I figured we'd want to continue support gllssical working on older data without requiring that it be re-spiceinited.

scsides · 2021-02-24T23:06:45Z

isis/src/galileo/objs/SsiCamera/SsiCamera.cpp

@@ -81,7 +81,7 @@ namespace Isis {
    // Get the start time in et
    PvlGroup inst = lab.findGroup("Instrument", Pvl::Traverse);

-    double et = iTime((QString)inst["StartTime"]).Et();
+    double et = getClockTime((QString)inst["SpacecraftClockStartCount"]).Et();


Is this still necessary when using setImage call to get sun distance?

Nope, thanks for catching this.

Kristin Berry added 2 commits February 22, 2021 13:17

Update gllssical to work without local spice kernels

1892973

getClockTime doesn't furnsh missing kernels

b3b43db

krlberry marked this pull request as draft February 22, 2021 20:31

krlberry commented Feb 22, 2021

View reviewed changes

isis/src/galileo/objs/SsiCamera/SsiCamera.cpp Outdated Show resolved Hide resolved

scsides approved these changes Feb 22, 2021

View reviewed changes

Move into try

e4ac325

scsides suggested changes Feb 24, 2021

View reviewed changes

Kristin Berry added 2 commits February 24, 2021 09:01

Updated to work on old spiceinited data and add test

f1212e9

A bit of cleanup

181c98d

krlberry commented Feb 24, 2021

View reviewed changes

Forgot first naif status

2e92673

krlberry requested a review from scsides February 24, 2021 16:31

krlberry marked this pull request as ready for review February 24, 2021 16:31

Update to use setImage to set the time of the camera

f51ebe5

scsides reviewed Feb 24, 2021

View reviewed changes

Updated based on feedback

6e629f0

krlberry requested a review from scsides February 25, 2021 15:39

scsides approved these changes Feb 25, 2021

View reviewed changes

krlberry merged commit 3812ae2 into DOI-USGS:dev Feb 25, 2021

krlberry mentioned this pull request Mar 4, 2021

Update shadow documentation to mention when SPICE kernels are needed #4335

Merged

13 tasks

krlberry mentioned this pull request Jun 17, 2021

Update Calibration Apps to Use Remote SPICE USGS-Astrogeology/softwaremanagement#34

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update gllssical to work without local spice kernels #4306

Update gllssical to work without local spice kernels #4306

krlberry commented Feb 22, 2021 •

edited

Loading

scsides left a comment

scsides commented Feb 22, 2021 •

edited

Loading

scsides Feb 24, 2021

krlberry Feb 24, 2021

scsides Feb 24, 2021

krlberry Feb 25, 2021

scsides Feb 24, 2021

krlberry Feb 25, 2021

scsides Feb 24, 2021

krlberry Feb 25, 2021 •

edited

Loading

Update gllssical to work without local spice kernels #4306

Update gllssical to work without local spice kernels #4306

Conversation

krlberry commented Feb 22, 2021 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Licensing

scsides left a comment

Choose a reason for hiding this comment

scsides commented Feb 22, 2021 • edited Loading

scsides Feb 24, 2021

Choose a reason for hiding this comment

krlberry Feb 24, 2021

Choose a reason for hiding this comment

scsides Feb 24, 2021

Choose a reason for hiding this comment

krlberry Feb 25, 2021

Choose a reason for hiding this comment

scsides Feb 24, 2021

Choose a reason for hiding this comment

krlberry Feb 25, 2021

Choose a reason for hiding this comment

scsides Feb 24, 2021

Choose a reason for hiding this comment

krlberry Feb 25, 2021 • edited Loading

Choose a reason for hiding this comment

krlberry commented Feb 22, 2021 •

edited

Loading

scsides commented Feb 22, 2021 •

edited

Loading

krlberry Feb 25, 2021 •

edited

Loading