DO NOT MERGE Adds SCCONFIG parameter to jigsaw #3612

paarongiroux · 2019-12-18T21:28:33Z

Description

Added SSCONFIG parameter to jigsaw.xml. This allows users to pass in a pvl file to set different parameters for different instruments. Added logic into jigsaw's main.cpp in order to facilitate the use of this new parameter.
Added a new BundleObservationSolveSettings constructor to build a BundleObservationSolveSettings given a pvl group.
Currently causing some existing tests to fail, and needs new tests to cover the changes we've implemented.

Related Issue

#3369

Motivation and Context

How Has This Been Tested?

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.

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.

paarongiroux · 2019-12-18T21:30:03Z

isis/src/control/apps/jigsaw/jigsaw.xml

          <item>OVEREXISTING</item>
          <item>SPKDEGREE</item>
          <item>SPKSOLVEDEGREE</item>
          <item>SPSOLVE</item>
+          <!-- <item>TWIST</item> -->


For some reason, having this leads to the following error message when running jigsaw:
**USER ERROR** Parameter [TWIST] must be used if parameter [SCCONFIG] is used.

paarongiroux · 2019-12-18T21:31:22Z

isis/src/control/apps/jigsaw/jigsaw.xml

-          CAMERA_ANGULAR_VELOCITY_SIGMA, CAMERA_ANGULAR_ACCELERATION_SIGMA. An example
-          template is located at $base/templates/jigsaw/SC_Parameters.pvl.
-        </description>
+      <parameter name="SCCONFIG">
        <exclusions>


currently, I've only added parameters that the BOSS constructor populates into the exclusion list.
I'm sure there are other parameters that should be excluded, but I have a hard time figuring out which ones.

…tory comments

jessemapel · 2019-12-19T18:03:54Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+      m_numberCamAngleCoefSolved = m_ckSolveDegree + 1;
+    }
+
+    m_anglesAprioriSigma.clear();


This should be moved down to where it is actually set.

is it even necessary to clear them? I'm guessing they are there from when this was implemented as the setFromPvl method, but since it's a constructor now this will be the first time that they are set anywhere

that's correct these can just go away then.

jessemapel · 2019-12-19T18:07:49Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+      m_numberCamPosCoefSolved = m_spkSolveDegree + 1;
+    }
+
+    m_positionAprioriSigma.clear();


This should also be moved down to where it is actually set.

jessemapel · 2019-12-19T18:11:56Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+     }
+    }
+
+    if (csolve != "NONE") {


This should be checking m_instrumentPointingSolveOption

jessemapel · 2019-12-19T18:13:51Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+           (double)(scParameterGroup.findKeyword("CAMERA_ANGULAR_ACCELERATION_SIGMA")));
+    }
+
+    if (ssolve != "NONE") {


Same here this should check m_instrumentPositionSolveOption

jessemapel · 2019-12-19T18:27:39Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+     if (scParameterGroup.hasKeyword("CAMERA_ANGULAR_ACCELERATION_SIGMA"))
+       m_anglesAprioriSigma.append(
+           (double)(scParameterGroup.findKeyword("CAMERA_ANGULAR_ACCELERATION_SIGMA")));
+    }


based on setInstrumentPointingSettings, the m_anglesAprioriSigma list needs have m_numberCamAngleCoefSolved elements and any sigmas that are not passed in should be set to ISIS::Null. So each of this ifs needs to have an else that appends a Null.

jessemapel · 2019-12-19T18:28:45Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+     if (scParameterGroup.hasKeyword("SPACECRAFT_VELOCITY_SIGMA"))
+       m_positionAprioriSigma.append((double)(scParameterGroup.findKeyword("SPACECRAFT_VELOCITY_SIGMA")));
+     if (scParameterGroup.hasKeyword("SPACECRAFT_ACCELERATION_SIGMA"))
+       m_positionAprioriSigma.append((double)(scParameterGroup.findKeyword("SPACECRAFT_ACCELERATION_SIGMA")));


Same as above, each if here should have an else that appends an ISIS::Null

jessemapel · 2019-12-19T18:34:10Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+     if (scParameterGroup.hasKeyword("CAMERA_ANGULAR_ACCELERATION_SIGMA"))
+       m_anglesAprioriSigma.append(
+           (double)(scParameterGroup.findKeyword("CAMERA_ANGULAR_ACCELERATION_SIGMA")));
+    }


This should also look for ADDITIONAL_CAMERA_POINTING_SIGMAS and append whatever is passed in to m_anglesAprioriSigma

jessemapel · 2019-12-19T18:34:39Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+     if (scParameterGroup.hasKeyword("SPACECRAFT_VELOCITY_SIGMA"))
+       m_positionAprioriSigma.append((double)(scParameterGroup.findKeyword("SPACECRAFT_VELOCITY_SIGMA")));
+     if (scParameterGroup.hasKeyword("SPACECRAFT_ACCELERATION_SIGMA"))
+       m_positionAprioriSigma.append((double)(scParameterGroup.findKeyword("SPACECRAFT_ACCELERATION_SIGMA")));


Same here, look for ADDITIONAL_SPACECRAFT_POSITION_SIGMAS and append those to m_positionAprioriSigma

Not sure how this would look. In the ADDITIONAL_SPACECRAFT_POSITION_SIGMAS keyword, would it contain a list of doubles and then I would loop through that list and append them all? I'm also guessing I don't need to append null if this keyword isn't found

It would be an array of doubles, you can use PvlKeyword::size and PvlKeyword::operator[](int index) to loop through the values and append them. It is purely for sigmas past the acceleration so if it's not there you don't need to pad.

jessemapel

New BOSS constructor also needs a test

jessemapel · 2019-12-19T20:07:59Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

-       m_anglesAprioriSigma.append((double)(scParameterGroup.findKeyword("CAMERA_ANGLES_SIGMA")));
-     if (scParameterGroup.hasKeyword("CAMERA_ANGULAR_VELOCITY_SIGMA"))
-       m_anglesAprioriSigma.append(
+    if (m_instrumentPointingSolveOption) {


This is an enumeration I would check m_instrumentPointingSolveOption != NoPointingFactors

jessemapel · 2019-12-19T20:08:17Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

-       m_positionAprioriSigma.append((double)(scParameterGroup.findKeyword("SPACECRAFT_VELOCITY_SIGMA")));
-     if (scParameterGroup.hasKeyword("SPACECRAFT_ACCELERATION_SIGMA"))
-       m_positionAprioriSigma.append((double)(scParameterGroup.findKeyword("SPACECRAFT_ACCELERATION_SIGMA")));
+    if (m_instrumentPositionSolveOption) {


Same as for pointing, check m_instrumentPositionSolveOption != NoPositionFactors

jessemapel · 2019-12-19T20:08:50Z

isis/src/control/objs/BundleUtilities/BundleObservationSolveSettings.cpp

+        for (int i = 0; i < additionalSigmas.size(); i++ ) {
+          m_positionAprioriSigma.append(toDouble(additionalSigmas[i]));
+        }
+      }


This is good, the pointing needs to have similar logic for ADDITIONAL_CAMERA_POINTING_SIGMAS

paarongiroux · 2019-12-20T22:54:25Z

Pushed my changes with the nearly complete gtest for the BundleObservationSolveSettings class.
the SaveSettings is missing some EXPECT statements for the instrument position options in the xml. I also didn't get around to writing a test for the constructor that takes in a Project object. Not really sure how I would have tested that anyways.

As for the jigsaw stuff, we still need to update truth data of existing tests and add a couple of new ones after this PR is merged.

… jigsaw

…igsaw

jessemapel · 2020-03-23T20:44:22Z

See #3792 for the continuation of this PR as Aaron is not working on it.

added SCCONFIG parameter to jigsaw

59e22be

paarongiroux commented Dec 18, 2019

View reviewed changes

paarongiroux added 2 commits December 19, 2019 11:08

fixed problem with how I was assigning observation numbers, added his…

8d1df51

…tory comments

fixed problem with how I was removing observation numbers

494f042

jessemapel reviewed Dec 19, 2019

View reviewed changes

cleaned up BOSS constructor

799e4b8

jessemapel reviewed Dec 19, 2019

View reviewed changes

paarongiroux added 2 commits December 19, 2019 13:20

more fixes to BOSS constructor

3da5649

added BOSS gtest

f047ac7

jessemapel and others added 6 commits December 20, 2019 17:03

Added jigsaw scconfig app test

e9be807

Added scconfig with heldlist app test

09d19af

more updates to xml save test

a4429b5

Merge branch 'dev' of https://github.com/USGS-Astrogeology/ISIS3 into…

188b619

… jigsaw

finished xml save test

9bf6ac2

Merge branch 'jigsaw' of https://github.com/paarongiroux/ISIS3 into j…

042258c

…igsaw

jessemapel mentioned this pull request Mar 23, 2020

Multisensor #3792

Merged

11 tasks

jessemapel closed this Mar 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DO NOT MERGE Adds SCCONFIG parameter to jigsaw #3612

DO NOT MERGE Adds SCCONFIG parameter to jigsaw #3612

paarongiroux commented Dec 18, 2019 •

edited

Loading

paarongiroux Dec 18, 2019 •

edited

Loading

paarongiroux Dec 18, 2019

jessemapel Dec 19, 2019

paarongiroux Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

paarongiroux Dec 19, 2019

jessemapel Dec 19, 2019 •

edited

Loading

jessemapel left a comment

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

jessemapel Dec 19, 2019

paarongiroux commented Dec 20, 2019

jessemapel commented Mar 23, 2020

***DO NOT MERGE*** Adds SCCONFIG parameter to jigsaw #3612

***DO NOT MERGE*** Adds SCCONFIG parameter to jigsaw #3612

Conversation

paarongiroux commented Dec 18, 2019 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Licensing

paarongiroux Dec 18, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jessemapel Dec 19, 2019 • edited Loading

Choose a reason for hiding this comment

jessemapel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paarongiroux commented Dec 20, 2019

jessemapel commented Mar 23, 2020

DO NOT MERGE Adds SCCONFIG parameter to jigsaw #3612

DO NOT MERGE Adds SCCONFIG parameter to jigsaw #3612

paarongiroux commented Dec 18, 2019 •

edited

Loading

paarongiroux Dec 18, 2019 •

edited

Loading

jessemapel Dec 19, 2019 •

edited

Loading