From e9e973a90700a4d87494ce3135aef59fef9ce2a1 Mon Sep 17 00:00:00 2001 From: kledmundson <6842706+kledmundson@users.noreply.github.com> Date: Fri, 25 Oct 2024 12:48:37 -0700 Subject: [PATCH] Bug fixes to address incorrect handling of RADIUS in the jigsaw GUI and in the bundleout.txt file when performing a rectangular (XYZ) bundle adjustment, originally implemented in UofA OSIRIS-REx code on 2019-07-30. Addresses #5642. (#5643) * The RADIUS checkbox in the GUI is excluded when a RECTANGULAR solution is selected. * In the bundleout.txt file, for rectangular solutions, 1) RADIUS is set to N/A in the SOLVE OPTIONS section; and 2) the POINTS UNCERTAINTY SECTION was fixed to properly display adjusted point uncertainty statistics with Error Propagation turned on. * Spacing for point labels was cleaned up in the INPUT: GLOBAL IMAGE PARAMETER UNCERTAINTIES section. * Finally, a slight modification was added to the FunctionalTestJigsawBundleXYZ ctest to verify that RADIUS is N/A in a RECTANGULAR solution. --- CHANGELOG.md | 2 + isis/src/control/apps/jigsaw/jigsaw.xml | 6 ++ .../objs/BundleAdjust/BundleAdjust.cpp | 30 ++++--- .../control/objs/BundleAdjust/BundleAdjust.h | 6 +- .../BundleSolutionInfo/BundleSolutionInfo.cpp | 83 ++++++++++--------- .../BundleSolutionInfo/BundleSolutionInfo.h | 9 ++ isis/tests/FunctionalTestsJigsaw.cpp | 10 +-- 7 files changed, 92 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e092d62e40..6edf242875 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ release. - Updated pixel2map documentation ### Fixed +- Fixed jigsaw bugs in which RADIUS is handled incorrectly in the jigsaw gui and in the bundleout.txt +file. Slightly modified the FunctionalTestJigsawBundleXYZ ctest accordingly. Issue: [5642](https://github.com/DOI-USGS/ISIS3/issues/5642) - Fixed a bug in isisminer in which bad (e.g. self-intersecting) polygon geometries were not treated properly. Added pertinent unit tests to GisGeometry and Strategy classes. Issue: [5612](https://github.com/DOI-USGS/ISIS3/issues/5612) - Fixed a bug in kaguyasp2isis that doesn't work for data with a detached label. diff --git a/isis/src/control/apps/jigsaw/jigsaw.xml b/isis/src/control/apps/jigsaw/jigsaw.xml index 08abf03688..1dc590d335 100644 --- a/isis/src/control/apps/jigsaw/jigsaw.xml +++ b/isis/src/control/apps/jigsaw/jigsaw.xml @@ -677,6 +677,11 @@ Fixed measure residual reporting in bundleout.txt file to match the residuals reported in the residuals CSV file. + + Modified xml so that the RADIUS on/off radio button is excluded when the + solution coordinate type is set to Rectangular. Originally added to UofA + code on 2019-07-30. + @@ -1601,6 +1606,7 @@ and reported in body-fixed rectangular coordinates (X, Y, and Z). + RADIUS POINT_LATITUDE_SIGMA POINT_LONGITUDE_SIGMA POINT_RADIUS_SIGMA diff --git a/isis/src/control/objs/BundleAdjust/BundleAdjust.cpp b/isis/src/control/objs/BundleAdjust/BundleAdjust.cpp index 08a6f0fcb2..23a66ca2bf 100644 --- a/isis/src/control/objs/BundleAdjust/BundleAdjust.cpp +++ b/isis/src/control/objs/BundleAdjust/BundleAdjust.cpp @@ -3264,7 +3264,17 @@ namespace Isis { Statistics sigmaCoord3Stats; Distance sigmaCoord1Dist, sigmaCoord2Dist, sigmaCoord3Dist; - SurfacePoint::CoordinateType type = m_bundleSettings->controlPointCoordTypeReports(); + SurfacePoint::CoordinateType reportType = m_bundleSettings->controlPointCoordTypeReports(); + SurfacePoint::CoordinateType bundleType = m_bundleSettings->controlPointCoordTypeBundle(); + + // we report statistics on coordinate 3 (Radius or Z) UNLESS + // bundle and report types are BOTH Latitudinal AND Radius is OFF + bool reportCoord3Stats = true; + if (bundleType == SurfacePoint::Latitudinal && + reportType == SurfacePoint::Latitudinal && + m_bundleSettings->solveRadius() == false) { + reportCoord3Stats = false; + } int numPoints = m_bundleControlPoints.size(); // initialize max and min values to those from first valid point @@ -3272,11 +3282,11 @@ namespace Isis { const BundleControlPointQsp point = m_bundleControlPoints.at(i); - maxSigmaCoord1Dist = point->adjustedSurfacePoint().GetSigmaDistance(type, + maxSigmaCoord1Dist = point->adjustedSurfacePoint().GetSigmaDistance(reportType, SurfacePoint::One); minSigmaCoord1Dist = maxSigmaCoord1Dist; - maxSigmaCoord2Dist = point->adjustedSurfacePoint().GetSigmaDistance(type, + maxSigmaCoord2Dist = point->adjustedSurfacePoint().GetSigmaDistance(reportType, SurfacePoint::Two); minSigmaCoord2Dist = maxSigmaCoord2Dist; @@ -3286,8 +3296,8 @@ namespace Isis { minSigmaCoord2PointId = maxSigmaCoord1PointId; // Get stats for coordinate 3 if used - if (m_bundleSettings->solveRadius() || type == SurfacePoint::Rectangular) { - maxSigmaCoord3Dist = point->adjustedSurfacePoint().GetSigmaDistance(type, + if (reportCoord3Stats) { + maxSigmaCoord3Dist = point->adjustedSurfacePoint().GetSigmaDistance(reportType, SurfacePoint::Three); minSigmaCoord3Dist = maxSigmaCoord3Dist; @@ -3301,11 +3311,11 @@ namespace Isis { const BundleControlPointQsp point = m_bundleControlPoints.at(i); - sigmaCoord1Dist = point->adjustedSurfacePoint().GetSigmaDistance(type, + sigmaCoord1Dist = point->adjustedSurfacePoint().GetSigmaDistance(reportType, SurfacePoint::One); - sigmaCoord2Dist = point->adjustedSurfacePoint().GetSigmaDistance(type, + sigmaCoord2Dist = point->adjustedSurfacePoint().GetSigmaDistance(reportType, SurfacePoint::Two); - sigmaCoord3Dist = point->adjustedSurfacePoint().GetSigmaDistance(type, + sigmaCoord3Dist = point->adjustedSurfacePoint().GetSigmaDistance(reportType, SurfacePoint::Three); sigmaCoord1Stats.AddData(sigmaCoord1Dist.meters()); @@ -3320,7 +3330,7 @@ namespace Isis { maxSigmaCoord2Dist = sigmaCoord2Dist; maxSigmaCoord2PointId = point->id(); } - if (m_bundleSettings->solveRadius() || type == SurfacePoint::Rectangular) { + if (reportCoord3Stats) { if (sigmaCoord3Dist > maxSigmaCoord3Dist) { maxSigmaCoord3Dist = sigmaCoord3Dist; maxSigmaCoord3PointId = point->id(); @@ -3334,7 +3344,7 @@ namespace Isis { minSigmaCoord2Dist = sigmaCoord2Dist; minSigmaCoord2PointId = point->id(); } - if (m_bundleSettings->solveRadius() || type == SurfacePoint::Rectangular) { + if (reportCoord3Stats) { if (sigmaCoord3Dist < minSigmaCoord3Dist) { minSigmaCoord3Dist = sigmaCoord3Dist; minSigmaCoord3PointId = point->id(); diff --git a/isis/src/control/objs/BundleAdjust/BundleAdjust.h b/isis/src/control/objs/BundleAdjust/BundleAdjust.h index b0131f1033..051d8e5822 100644 --- a/isis/src/control/objs/BundleAdjust/BundleAdjust.h +++ b/isis/src/control/objs/BundleAdjust/BundleAdjust.h @@ -319,12 +319,16 @@ namespace Isis { * @history 2018-11-29 Ken Edmundson - Modifed init, initializeNormalEquationsMatrix, and * computePartials methods. * @history 2019-04-29 Ken Edmundson - Modifications for bundle with lidar. - * @history 2019-05-15 Debbie A. Cook - The call to CameraGroundMap::GetXY in method + * @history 2019-05-15 Debbie A. Cook - The call to CameraGroundMap::GetXY in method * ComputePartials was modified to not check for points on the back side * of the planet when computing instrument coordinates during the bundle * adjustment. In the future a control net diagnostic program might be * useful to detect any points not visible on an image based on the exterior * orientation of the image. References #2591. + * @history 2024-10-21 Ken Edmundson - Simplified the computation of statistics for coordinate 3 + * (Radius or Z), to ensure correct output in the bundleout.txt file. We + * report statistics on coordinate 3 UNLESS bundle and report types are + * BOTH Latitudinal AND Radius is OFF. */ class BundleAdjust : public QObject { Q_OBJECT diff --git a/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.cpp b/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.cpp index 29245fe798..ed8fc99afc 100755 --- a/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.cpp +++ b/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.cpp @@ -662,9 +662,14 @@ namespace Isis { snprintf(buf, sizeof(buf), "\n OBSERVATIONS: OFF"); fpOut << buf; - m_settings->solveRadius() ? - snprintf(buf, sizeof(buf), "\n RADIUS: ON"): - snprintf(buf, sizeof(buf), "\n RADIUS: OFF"); + if (m_settings->controlPointCoordTypeBundle() == SurfacePoint::Latitudinal) { + m_settings->solveRadius() ? + snprintf(buf, sizeof(buf), "\n RADIUS: ON"): + snprintf(buf, sizeof(buf), "\n RADIUS: OFF"); + } + else { // Rectangular (XYZ) solution + snprintf(buf, sizeof(buf), "\n RADIUS: N/A"); + } fpOut << buf; m_settings->solveTargetBody() ? @@ -833,16 +838,16 @@ namespace Isis { QString coord1Str; QString coord2Str; QString coord3Str; - switch (m_settings->controlPointCoordTypeReports()) { + switch (m_settings->controlPointCoordTypeBundle()) { case SurfacePoint::Latitudinal: - coord1Str = "LATITUDE"; - coord2Str = "LONGITUDE"; - coord3Str = "RADIUS"; + coord1Str = " POINT LATITUDE"; + coord2Str = " POINT LONGITUDE"; + coord3Str = " POINT RADIUS"; break; case SurfacePoint::Rectangular: - coord1Str = " X"; - coord2Str = " Y"; - coord3Str = " Z"; + coord1Str = " POINT X"; + coord2Str = " POINT Y"; + coord3Str = " POINT Z"; break; default: IString msg ="Unknown surface point coordinate type enum [" @@ -854,20 +859,20 @@ namespace Isis { // Coordinate 1 (latitude or point X) fpOut << buf; (m_settings->globalPointCoord1AprioriSigma() == Isis::Null) ? - snprintf(buf, sizeof(buf),"\n POINT %s SIGMA: N/A", coord1Str.toLatin1().data()): - snprintf(buf, sizeof(buf),"\n POINT %s SIGMA: %lf (meters)", coord1Str.toLatin1().data(), + snprintf(buf, sizeof(buf),"\n%s SIGMA: N/A", coord1Str.toLatin1().data()): + snprintf(buf, sizeof(buf),"\n%s SIGMA: %lf (meters)", coord1Str.toLatin1().data(), m_settings->globalPointCoord1AprioriSigma()); // Coordinate 2 (longitude or point Y) fpOut << buf; (m_settings->globalPointCoord2AprioriSigma() == Isis::Null) ? - snprintf(buf, sizeof(buf),"\n POINT %s SIGMA: N/A", coord2Str.toLatin1().data()): - snprintf(buf, sizeof(buf),"\n POINT %s SIGMA: %lf (meters)", coord2Str.toLatin1().data(), - m_settings->globalPointCoord2AprioriSigma()); + snprintf(buf, sizeof(buf),"\n%s SIGMA: N/A", coord2Str.toLatin1().data()): + snprintf(buf, sizeof(buf),"\n%s SIGMA: %lf (meters)", coord2Str.toLatin1().data(), + m_settings->globalPointCoord2AprioriSigma()); // Coordinate 3 (radius or point Z) fpOut << buf; (m_settings->globalPointCoord3AprioriSigma() == Isis::Null) ? - snprintf(buf, sizeof(buf),"\n POINT %s SIGMA: N/A", coord3Str.toLatin1().data()): - snprintf(buf, sizeof(buf),"\n POINT %s SIGMA: %lf (meters)", coord3Str.toLatin1().data(), + snprintf(buf, sizeof(buf),"\n%s SIGMA: N/A", coord3Str.toLatin1().data()): + snprintf(buf, sizeof(buf),"\n%s SIGMA: %lf (meters)", coord3Str.toLatin1().data(), m_settings->globalPointCoord3AprioriSigma()); fpOut << buf; (positionSolveDegree < 1 || positionSigmas[0] == Isis::Null) ? @@ -1461,9 +1466,8 @@ namespace Isis { fpOut << buf; // Coordinate 1 (latitude or point x) summary - QString - coordName = surfacePointCoordName(m_settings->controlPointCoordTypeReports(), - SurfacePoint::One); + QString coordName = surfacePointCoordName(m_settings->controlPointCoordTypeReports(), + SurfacePoint::One); snprintf(buf, sizeof(buf), "RMS Sigma %s(m)%20.8lf\n", coordName.toLatin1().data(), m_statisticsResults->sigmaCoord1StatisticsRms()); fpOut << buf; @@ -1494,26 +1498,29 @@ namespace Isis { // Coordinate 3 (radius or point z) summary coordName = surfacePointCoordName(m_settings->controlPointCoordTypeReports(), SurfacePoint::Three); - if ( m_settings->solveRadius() ) { - snprintf(buf, sizeof(buf), "RMS Sigma %s(m)%20.8lf\n", coordName.toLatin1().data(), - m_statisticsResults->sigmaCoord3StatisticsRms()); - fpOut << buf; - snprintf(buf, sizeof(buf), "MIN Sigma %s(m)%20.8lf at %s\n", coordName.toLatin1().data(), - m_statisticsResults->minSigmaCoord3Distance().meters(), - m_statisticsResults->minSigmaCoord3PointId().toLatin1().data()); - fpOut << buf; - snprintf(buf, sizeof(buf), "MAX Sigma %s(m)%20.8lf at %s\n", coordName.toLatin1().data(), - m_statisticsResults->maxSigmaCoord3Distance().meters(), - m_statisticsResults->maxSigmaCoord3PointId().toLatin1().data()); - fpOut << buf; + + if (m_settings->controlPointCoordTypeBundle() == SurfacePoint::Latitudinal && + m_settings->controlPointCoordTypeReports() == SurfacePoint::Latitudinal && + m_settings->solveRadius() == false ) { + snprintf(buf, sizeof(buf), " RMS Sigma Radius(m) N/A\n"); + fpOut << buf; + snprintf(buf, sizeof(buf), " MIN Sigma Radius(m) N/A\n"); + fpOut << buf; + snprintf(buf, sizeof(buf), " MAX Sigma Radius(m) N/A\n"); + fpOut << buf; } else { - snprintf(buf, sizeof(buf), " RMS Sigma Radius(m) N/A\n"); - fpOut << buf; - snprintf(buf, sizeof(buf), " MIN Sigma Radius(m) N/A\n"); - fpOut << buf; - snprintf(buf, sizeof(buf), " MAX Sigma Radius(m) N/A\n"); - fpOut << buf; + snprintf(buf, sizeof(buf), "RMS Sigma %s(m)%20.8lf\n", coordName.toLatin1().data(), + m_statisticsResults->sigmaCoord3StatisticsRms()); + fpOut << buf; + snprintf(buf, sizeof(buf), "MIN Sigma %s(m)%20.8lf at %s\n", coordName.toLatin1().data(), + m_statisticsResults->minSigmaCoord3Distance().meters(), + m_statisticsResults->minSigmaCoord3PointId().toLatin1().data()); + fpOut << buf; + snprintf(buf, sizeof(buf), "MAX Sigma %s(m)%20.8lf at %s\n", coordName.toLatin1().data(), + m_statisticsResults->maxSigmaCoord3Distance().meters(), + m_statisticsResults->maxSigmaCoord3PointId().toLatin1().data()); + fpOut << buf; } } diff --git a/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.h b/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.h index d5ef9eafb5..6ba6d9540c 100755 --- a/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.h +++ b/isis/src/control/objs/BundleSolutionInfo/BundleSolutionInfo.h @@ -155,6 +155,15 @@ namespace Isis { * from ISIS3 because it had become unmaintainable. * @history 2019-06-03 Adam Paquette - Updated the header for the bundleout.txt file for * more human readable formatting in the bundleout.txt file. + * @history 2024-10-21 Ken Edmundson - 1) Fixed bug in the outputText method when error + * propagation is on in a rectangular (XYZ) solution. In the "POINTS + * UNCERTAINTY SUMMARY" section, instead of writing the RMS of + * Z-coordinate sigmas, the RMS, MIN, and MAX of radius coordinates + * was reported as "N/A"; 2) Modified outputHeader method + * to output RADIUS: N/A under "INPUT: SOLVE OPTIONS" in a rectangular + * solution. 3) Cleaned up spacing of Point Coordinate output in the + * "INPUT: GLOBAL IMAGE PARAMETER UNCERTAINTIES" section. Originally + * added to UofA code on 2019-07-30. * */ class BundleSolutionInfo : public QObject { diff --git a/isis/tests/FunctionalTestsJigsaw.cpp b/isis/tests/FunctionalTestsJigsaw.cpp index 61d957a41c..6135b34b31 100644 --- a/isis/tests/FunctionalTestsJigsaw.cpp +++ b/isis/tests/FunctionalTestsJigsaw.cpp @@ -273,8 +273,7 @@ TEST_F(ApolloNetwork, FunctionalTestJigsawBundleXYZ) { // Rectangular Bundle, Latitudinal output - QVector args3 = {"radius=yes", - "errorpropagation=yes", + QVector args3 = {"errorpropagation=yes", "spsolve=position", "spacecraft_position_sigma=1000.0", "camsolve=angles", @@ -292,7 +291,7 @@ TEST_F(ApolloNetwork, FunctionalTestJigsawBundleXYZ) { UserInterface ui3(APP_XML, args3); jigsaw(ui3); - // Compare newtwork and images.csv against the latitude, latitude bundle + // Compare network and images.csv against the latitude, latitude bundle // Compare network against the latitude/latitude network ControlNet latLatNet(tempDir.path()+"/latlat_out.net"); @@ -358,6 +357,7 @@ TEST_F(ApolloNetwork, FunctionalTestJigsawBundleXYZ) { bundleFile2.close(); lines = bundleOut2.split("\n"); + EXPECT_THAT(lines[20].toStdString(), HasSubstr("N/A")); // radius is N/A in XYZ solution EXPECT_THAT(lines[24].toStdString(), HasSubstr("RECTANGULAR")); EXPECT_THAT(lines[58].toStdString(), HasSubstr("X")); EXPECT_THAT(lines[59].toStdString(), HasSubstr("Y")); @@ -372,7 +372,7 @@ TEST_F(ApolloNetwork, FunctionalTestJigsawBundleXYZ) { EXPECT_THAT(lines[670].toStdString(), HasSubstr("BODY-FIXED-Z")); - // Compare newtwork and images.csv against the rectangular, latitude bundle + // Compare network and images.csv against the rectangular, latitude bundle // Compare network against the rect/lat network ControlNet rectRectNet(tempDir.path()+"/rectlat_out.net"); @@ -451,7 +451,7 @@ TEST_F(ApolloNetwork, FunctionalTestJigsawBundleXYZ) { bundleFile4.close(); - // Compare newtwork and images.csv against the latitude, latitude bundle + // Compare network and images.csv against the latitude, latitude bundle // Compare network against the lat/lat network ControlNet latRectNet(tempDir.path()+"/rectlat_out.net");