From 6c2eda5184bd239dea8889c54b9842fbaedf134d Mon Sep 17 00:00:00 2001
From: Oliver Sander <oliver.sander@tu-dresden.de>
Date: Fri, 11 Oct 2019 21:49:15 +0200
Subject: [PATCH] harmonicenergytest.cc: Only test with points that are
 mutually close

Previously, this test used a situation with geodesic interpolation
between two points on the sphere that are almost antipodal.
This made the TargetSpaceRiemannianTRSolver crash, because the
weighted sum of squared distances can have indefinite second
derivatives in such a situation.  In principle, the
TargetSpaceRiemannianTRSolver should handle this, but nowadays
it is not actually a TR solver anymore, but a plain local
Newton method.  Maybe I should revert back to trust-region.

I added a comment to targetspacetrsolver.cc that explains the
situation a little.
---
 dune/gfe/targetspacertrsolver.cc |  9 +++++++++
 test/harmonicenergytest.cc       | 15 +++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/dune/gfe/targetspacertrsolver.cc b/dune/gfe/targetspacertrsolver.cc
index b00b59e1..7af60d32 100644
--- a/dune/gfe/targetspacertrsolver.cc
+++ b/dune/gfe/targetspacertrsolver.cc
@@ -55,6 +55,15 @@ void TargetSpaceRiemannianTRSolver<TargetSpace>::solve()
         // /////////////////////////////
         //    Solve !
         // /////////////////////////////
+        // TODO: The GramSchmidtSolver may not be the smartest choice here, because it needs
+        // a matrix that is positive definite on the complement of its kernel.  This can limit
+        // the radius of convergence of the Newton solver.  As an example consider interpolation
+        // between two points on the unit sphere that are more than pi/2 apart.  There is a
+        // well-defined unique shortest geodesic between two such points, but depending on the
+        // weights, the weighted sum of squared distances does not everywhere have a positive definite
+        // second derivative.
+        // Maybe the Newton solver has to be replaced by a trust-region or line-search solver
+        // for such situations.
         Dune::FieldMatrix<field_type,blocksize,embeddedBlocksize> basis = x_.orthonormalFrame();
         GramSchmidtSolver<field_type, blocksize, embeddedBlocksize>::solve(hesseMatrix, corr, rhs, basis);
 
diff --git a/test/harmonicenergytest.cc b/test/harmonicenergytest.cc
index a6e3b3de..21e418a1 100644
--- a/test/harmonicenergytest.cc
+++ b/test/harmonicenergytest.cc
@@ -17,6 +17,17 @@ typedef UnitVector<double,3> TargetSpace;
 
 using namespace Dune;
 
+/** \brief Computes the diameter of a set */
+template <class TargetSpace>
+double diameter(const std::vector<TargetSpace>& v)
+{
+    double d = 0;
+    for (size_t i=0; i<v.size(); i++)
+        for (size_t j=0; j<v.size(); j++)
+            d = std::max(d, TargetSpace::distance(v[i],v[j]));
+    return d;
+}
+
 template <class Basis>
 void testEnergy(const Basis& basis, const std::vector<TargetSpace>& coefficients)
 {
@@ -102,6 +113,10 @@ void testUnitVector3d()
         for (int j=0; j<dim+1; j++)
             coefficients[j] = testPoints[index[j]];
 
+        // This may be overly restrictive, but see the TODO in targetspacetrsolver.cc
+        if (diameter(coefficients) > TargetSpace::convexityRadius)
+            continue;
+
         testEnergy<Basis>(basis, coefficients);
         
     }
-- 
GitLab