Fix issues regarding zero length V3D

Most issues stemmed from trying to normalize a zero length V3D in the
unit tests. We now throw in a few places where an illegal vector
operation is tried out.

Re #24898

Framework/API/src/DetectorSearcher.cpp

@@ -75,7 +75,12 @@ void DetectorSearcher::createDetectorCache() {
     auto pos = m_detInfo.position(pointNo);
     pos.normalize();
     auto E1 = (pos - beam) * -m_crystallography_convention;
-    E1.normalize();
+    const auto norm = E1.norm();
+    if (norm == 0.) {
+      E1 = V3D(up) * -m_crystallography_convention;
+    } else {
+      E1 /= norm;
+    }
 
     Eigen::Vector3d point(E1[0], E1[1], E1[2]);
 

Framework/DataHandling/src/LoadIsawDetCal.cpp

@@ -441,28 +441,33 @@ void LoadIsawDetCal::doRotation(V3D rX, V3D rY, ComponentInfo &componentInfo,
   rY.normalize();
 
   // These are the original axes
-  const V3D oX(1., 0., 0.);
-  const V3D oY(0., 1., 0.);
+  constexpr V3D oX(1., 0., 0.);
+  constexpr V3D oY(0., 1., 0.);
 
   // Axis that rotates X
-  V3D ax1 = oX.cross_prod(rX);
-  // Rotation angle from oX to rX
-  double angle1 = oX.angle(rX) * DegreesPerRadian;
-  if (doWishCorrection)
-    angle1 += 180.0;
-  // Create the first quaternion
-  Quat Q1(angle1, ax1);
+  const V3D ax1 = oX.cross_prod(rX);
+  Quat Q1;
+  if (!ax1.nullVector(1e-12)) {
+    // Rotation angle from oX to rX
+    double angle1 = oX.angle(rX) * DegreesPerRadian;
+    if (doWishCorrection)
+      angle1 += 180.0;
+    // Create the first quaternion
+    Q1.setAngleAxis(angle1, ax1);
+  }
 
   // Now we rotate the original Y using Q1
   V3D roY = oY;
   Q1.rotate(roY);
   // Find the axis that rotates oYr onto rY
-  V3D ax2 = roY.cross_prod(rY);
-  const double angle2 = roY.angle(rY) * DegreesPerRadian;
-  Quat Q2(angle2, ax2);
-
+  const V3D ax2 = roY.cross_prod(rY);
+  Quat Q2;
+  if (!ax2.nullVector(1e-12)) {
+    const double angle2 = roY.angle(rY) * DegreesPerRadian;
+    Q2.setAngleAxis(angle2, ax2);
+  }
   // Final = those two rotations in succession; Q1 is done first.
-  Quat Rot = Q2 * Q1;
+  const Quat Rot = Q2 * Q1;
 
   // Then find the corresponding relative position
   const auto componentIndex = componentInfo.indexOf(comp->getComponentID());

Framework/DataHandling/test/LoadIsawDetCalTest.h

@@ -55,7 +55,6 @@ public:
     if (det != nullptr) {
       const auto detPos = det->getPos();
       const V3D testPos(x, y, z);
-      std::cout << detPos << " " << testPos << std::endl;
       TS_ASSERT_EQUALS(detPos, testPos);
     } else {
       throw std::runtime_error("In checkPosition IComponent is nullptr");
@@ -67,7 +66,6 @@ public:
     if (det != nullptr) {
       const auto detRot = det->getRotation();
       const Quat testRot(w, a, b, c);
-      std::cout << detRot << " " << testRot << std::endl;
       TS_ASSERT_EQUALS(detRot, testRot);
     } else {
       throw std::runtime_error("In checkRotation IComponent is nullptr");

Framework/Geometry/src/Crystal/OrientedLattice.cpp

@@ -191,11 +191,14 @@ const DblMatrix &OrientedLattice::setUFromVectors(const V3D &u, const V3D &v) {
     throw std::invalid_argument("|B.v|~0");
   buVec.normalize(); // 1st unit vector, along Bu
   V3D bwVec = buVec.cross_prod(bvVec);
-  if (bwVec.normalize() < 1e-5)
-    throw std::invalid_argument(
-        "u and v are parallel"); // 3rd unit vector, perpendicular to Bu,Bv
-  bvVec = bwVec.cross_prod(
-      buVec); // 2nd unit vector, perpendicular to Bu, in the Bu,Bv plane
+  const auto norm = bwVec.norm();
+  if (norm < 1e-5) {
+    // 3rd unit vector, perpendicular to Bu,Bv
+    throw std::invalid_argument("u and v are parallel");
+  }
+  bwVec /= norm;
+  // 2nd unit vector, perpendicular to Bu, in the Bu,Bv plane
+  bvVec = bwVec.cross_prod(buVec);
   DblMatrix tau(3, 3), lab(3, 3), U(3, 3);
   /*lab      = U tau
   / 0 1 0 \     /bu[0] bv[0] bw[0]\

Framework/Geometry/src/Instrument/InstrumentDefinitionParser.cpp

@@ -2021,11 +2021,9 @@ void InstrumentDefinitionParser::makeXYplaneFaceComponent(
 
 //-----------------------------------------------------------------------------------------------------------------------
 /** Make the shape defined in 1st argument face the position in the second
- *argument,
- *  by rotating the z-axis of the component passed in 1st argument so that it
- *points in the
- *  direction: from the position (as specified 2nd argument) to the component
- *(1st argument).
+ * argument, by rotating the z-axis of the component passed in 1st argument so
+ * that it points in the direction: from the position (as specified 2nd
+ * argument) to the component (1st argument).
  *
  *  @param in ::  Component to be rotated
  *  @param facingPoint :: position to face
@@ -2036,22 +2034,22 @@ void InstrumentDefinitionParser::makeXYplaneFaceComponent(
 
   // vector from facing object to component we want to rotate
   Kernel::V3D facingDirection = pos - facingPoint;
-  facingDirection.normalize();
-
   if (facingDirection.norm() == 0.0)
     return;
+  facingDirection.normalize();
 
   // now aim to rotate shape such that the z-axis of of the object we want to
-  // rotate
-  // points in the direction of facingDirection. That way the XY plane faces
-  // the
-  // 'facing object'.
-  Kernel::V3D z = Kernel::V3D(0, 0, 1);
+  // rotate points in the direction of facingDirection. That way the XY plane
+  // faces the 'facing object'.
+  constexpr Kernel::V3D z(0, 0, 1);
   Kernel::Quat R = in->getRotation();
   R.inverse();
   R.rotate(facingDirection);
 
   Kernel::V3D normal = facingDirection.cross_prod(z);
+  if (normal.norm() == 0.) {
+    normal = -facingDirection;
+  }
   normal.normalize();
   double theta = (180.0 / M_PI) * facingDirection.angle(z);
 
@@ -2059,8 +2057,7 @@ void InstrumentDefinitionParser::makeXYplaneFaceComponent(
     in->rotate(Kernel::Quat(-theta, normal));
   else {
     // To take into account the case where the facing direction is in the
-    // (0,0,1)
-    // or (0,0,-1) direction.
+    // (0,0,1) or (0,0,-1) direction.
     in->rotate(Kernel::Quat(-theta, Kernel::V3D(0, 1, 0)));
   }
 }

Framework/Geometry/src/Instrument/ObjComponent.cpp

@@ -115,11 +115,11 @@ int ObjComponent::interceptSurface(Track &track) const {
 
   // TODO: If scaling parameters are ever enabled, would they need need to be
   // used here?
-  V3D trkStart = factorOutComponentPosition(track.startPoint());
-  V3D trkDirection = takeOutRotation(track.direction());
+  const V3D trkStart = factorOutComponentPosition(track.startPoint());
+  const V3D trkDirection = takeOutRotation(track.direction());
 
   Track probeTrack(trkStart, trkDirection);
-  int intercepts = shape()->interceptSurface(probeTrack);
+  const int intercepts = shape()->interceptSurface(probeTrack);
 
   Track::LType::const_iterator it;
   for (it = probeTrack.cbegin(); it != probeTrack.cend(); ++it) {

Framework/Geometry/src/Objects/Track.cpp

@@ -22,7 +22,7 @@ using Kernel::V3D;
 /**
  * Default constructor
  */
-Track::Track() : m_startPoint(), m_unitVector() {}
+Track::Track() : m_startPoint(), m_unitVector(0., 0., 1.) {}
 
 /**
  * Constructor
@@ -30,7 +30,11 @@ Track::Track() : m_startPoint(), m_unitVector() {}
  * @param unitVector :: Directional vector. It must be unit vector.
  */
 Track::Track(const V3D &startPt, const V3D &unitVector)
-    : m_startPoint(startPt), m_unitVector(unitVector) {}
+    : m_startPoint(startPt), m_unitVector(unitVector) {
+  if (!unitVector.unitVector()) {
+    throw std::invalid_argument("Failed to construct track: direction is not a unit vector.");
+  }
+}
 
 /**
  * Resets the track starting point and direction.
@@ -38,6 +42,9 @@ Track::Track(const V3D &startPt, const V3D &unitVector)
  * @param direction :: The new direction. Must be a unit vector!
  */
 void Track::reset(const V3D &startPoint, const V3D &direction) {
+  if (!direction.unitVector()) {
+    throw std::invalid_argument("Failed to reset track: direction is not a unit vector.");
+  }
   m_startPoint = startPoint;
   m_unitVector = direction;
 }

Framework/Geometry/test/BoundingBoxTest.h

@@ -154,17 +154,25 @@ public:
         true);
 
     // Mix
+    V3D dir(1., 1., 0);
+    dir.normalize();
     TS_ASSERT_EQUALS(
-        bbox.doesLineIntersect(Track(V3D(-5.0, -1.0, 0.0), V3D(1.0, 1.0, 0.0))),
+        bbox.doesLineIntersect(Track(V3D(-5.0, -1.0, 0.0), dir)),
         true);
+    dir = {1., 1., 1.,};
+    dir.normalize();
     TS_ASSERT_EQUALS(bbox.doesLineIntersect(
-                         Track(V3D(-5.0, -1.0, -0.5), V3D(1.0, 1.0, 1.0))),
+                         Track(V3D(-5.0, -1.0, -0.5), dir)),
                      true);
+    dir = {-1., -0.4, 0.};
+    dir.normalize();
     TS_ASSERT_EQUALS(bbox.doesLineIntersect(
-                         Track(V3D(10.0, 10.0, 0.0), V3D(-1.0, -0.4, 0.0))),
+                         Track(V3D(10.0, 10.0, 0.0), dir)),
                      false);
+    dir = {1., 1., 0.};
+    dir.normalize();
     TS_ASSERT_EQUALS(bbox.doesLineIntersect(
-                         Track(V3D(-10.0, -10.0, 0.0), V3D(1.0, 1.0, 0.0))),
+                         Track(V3D(-10.0, -10.0, 0.0), dir)),
                      true); // Hits box at edge
   }
 

Framework/Geometry/test/CSGObjectTest.h

@@ -390,7 +390,9 @@ public:
     std::vector<Link>
         expectedResults; // left empty as there are no expected results
     auto geom_obj = createCappedCylinder();
-    Track track(V3D(-10, 0, 0), V3D(1, 1, 0));
+    V3D dir(1., 1., 0.);
+    dir.normalize();
+    Track track(V3D(-10, 0, 0), dir);
 
     checkTrackIntercept(geom_obj, track, expectedResults);
   }

Framework/Geometry/test/CylinderTest.h

@@ -189,10 +189,10 @@ public:
     Cylinder A;
     A.setSurface("cx 5");
 
-    TS_ASSERT_EQUALS(A.surfaceNormal(V3D(10, 0, 0)), V3D(0, 0, 0));
+    TS_ASSERT_THROWS_ANYTHING(A.surfaceNormal(V3D(10, 0, 0)));
     TS_ASSERT_EQUALS(A.surfaceNormal(V3D(0, 10, 0)), V3D(0, 1, 0));
     TS_ASSERT_EQUALS(A.surfaceNormal(V3D(0, 0, 10)), V3D(0, 0, 1));
-    TS_ASSERT_EQUALS(A.surfaceNormal(V3D(-10, 0, 0)), V3D(0, 0, 0));
+    TS_ASSERT_THROWS_ANYTHING(A.surfaceNormal(V3D(-10, 0, 0)));
     TS_ASSERT_EQUALS(A.surfaceNormal(V3D(0, -10, 0)), V3D(0, -1, 0));
     TS_ASSERT_EQUALS(A.surfaceNormal(V3D(0, 0, -10)), V3D(0, 0, -1));
 

Framework/Geometry/test/InstrumentDefinitionParserTest.h

@@ -258,8 +258,8 @@ public:
     TS_ASSERT(!ptrDet4->isValid(V3D(-0.02, 0.0, 0.0) + ptrDet4->getPos()));
     TS_ASSERT(!ptrDet4->isValid(V3D(0.0, 0.02, 0.0) + ptrDet4->getPos()));
     TS_ASSERT(!ptrDet4->isValid(V3D(0.0, -0.02, 0.0) + ptrDet4->getPos()));
-    TS_ASSERT(!ptrDet4->isValid(V3D(0.0, 0.0, 0.02) + ptrDet4->getPos()));
-    TS_ASSERT(ptrDet4->isValid(V3D(0.0, 0.0, -0.02) + ptrDet4->getPos()));
+    TS_ASSERT(ptrDet4->isValid(V3D(0.0, 0.0, 0.02) + ptrDet4->getPos()));
+    TS_ASSERT(!ptrDet4->isValid(V3D(0.0, 0.0, -0.02) + ptrDet4->getPos()));
 
     // test of facing as a sub-element of location
     boost::shared_ptr<const IDetector> ptrDet5 = i->getDetector(5);

Framework/Geometry/test/InstrumentRayTracerTest.h

@@ -120,6 +120,7 @@ public:
     Instrument_sptr testInst = setupInstrument();
     InstrumentRayTracer tracker(testInst);
     V3D testDir(0.010, 0.0, 15.004);
+    testDir.normalize();
     tracker.trace(testDir);
     Links results = tracker.getResults();
     TS_ASSERT_EQUALS(results.size(), 1);
@@ -156,9 +157,13 @@ public:
    */
   void doTestRectangularDetector(std::string message, Instrument_sptr inst,
                                  V3D testDir, int expectX, int expectY) {
-    //    std::cout << message << '\n';
     InstrumentRayTracer tracker(inst);
-    testDir.normalize(); // Force to be unit vector
+    const auto norm = testDir.norm();
+    if (norm == 0.) {
+      TS_ASSERT_THROWS_ANYTHING(tracker.traceFromSample(testDir));
+      return;
+    }
+    testDir /= norm;
     tracker.traceFromSample(testDir);
 
     Links results = tracker.getResults();

Framework/Geometry/test/MeshObjectTest.h

@@ -320,7 +320,9 @@ public:
     std::vector<Link>
         expectedResults; // left empty as there are no expected results
     auto geom_obj = createCube(4.0);
-    Track track(V3D(-10, 0, 0), V3D(1, 1, 0));
+    V3D dir(1., 1., 0.);
+    dir.normalize();
+    Track track(V3D(-10, 0, 0), dir);
 
     checkTrackIntercept(std::move(geom_obj), track, expectedResults);
   }
@@ -361,7 +363,9 @@ public:
   void testInterceptLShapeTwoPass() {
     std::vector<Link> expectedResults;
     auto geom_obj = createLShape();
-    Track track(V3D(0, 2.5, 0.5), V3D(0.707, -0.707, 0));
+    V3D dir(1., -1., 0.);
+    dir.normalize();
+    Track track(V3D(0, 2.5, 0.5), dir);
 
     // format = startPoint, endPoint, total distance so far
     expectedResults.emplace_back(

Framework/Geometry/test/ObjComponentTest.h

@@ -164,8 +164,10 @@ public:
     ocyl.setParent(&parent);
     // Check original track misses
     TS_ASSERT_EQUALS(ocyl.interceptSurface(track), 0);
-    // Create a new test track going from the origin down the line y = -x
-    Track track2(V3D(0, 0, 0), V3D(0, 1, -1));
+    // Create a new test track going from the origin down the line z = -y
+    V3D dir(0., 1., -1.);
+    dir.normalize();
+    Track track2(V3D(0, 0, 0), dir);
     TS_ASSERT_EQUALS(ocyl.interceptSurface(track2), 1);
     Track::LType::const_iterator it2 = track2.cbegin();
     if (it2 == track2.cend())
@@ -347,14 +349,14 @@ public:
     TS_ASSERT_EQUALS(itscale->entryPoint, V3D(-6.4, 0.0, 0.0));
     TS_ASSERT_EQUALS(itscale->exitPoint, V3D(2.4, 0.0, 0.0));
 
-    Track trackScaleY(V3D(0.0, -2, 0), V3D(0, 2.0, 0));
+    Track trackScaleY(V3D(0.0, -2, 0), V3D(0, 1.0, 0));
     TS_ASSERT_EQUALS(ocyl->interceptSurface(trackScaleY), 1);
     Track::LType::const_iterator itscaleY = trackScaleY.cbegin();
     TS_ASSERT_EQUALS(itscaleY->distFromStart, 2.5);
     TS_ASSERT_EQUALS(itscaleY->entryPoint, V3D(0.0, -0.5, 0.0));
     TS_ASSERT_EQUALS(itscaleY->exitPoint, V3D(0.0, 0.5, 0.0));
 
-    Track trackScaleW(V3D(0, 0, -5), V3D(0, 0, 5));
+    Track trackScaleW(V3D(0, 0, -5), V3D(0, 0, 1));
     TS_ASSERT_EQUALS(ocyl->interceptSurface(trackScaleW), 1);
     Track::LType::const_iterator itscaleW = trackScaleW.cbegin();
     TS_ASSERT_DELTA(itscaleW->distFromStart, 6.5, 1e-6);

Framework/Geometry/test/ParObjComponentTest.h

@@ -176,8 +176,10 @@ public:
     ocyl.setParent(&parent);
     // Check original track misses
     TS_ASSERT_EQUALS(pocyl.interceptSurface(track), 0);
-    // Create a new test track going from the origin down the line y = -x
-    Track track2(V3D(0, 0, 0), V3D(0, 1, -1));
+    // Create a new test track going from the origin down the line z = -y
+    V3D dir(0, 1., -1.);
+    dir.normalize();
+    Track track2(V3D(0, 0, 0), dir);
     TS_ASSERT_EQUALS(pocyl.interceptSurface(track2), 1);
     Track::LType::const_iterator it2 = track2.cbegin();
     if (it2 == track2.cend())

Framework/Kernel/inc/MantidKernel/Quat.h

@@ -113,7 +113,7 @@ public:
   Quat &operator*=(const Quat &);
   bool operator==(const Quat &) const;
   bool operator!=(const Quat &) const;
-  const double &operator[](int) const;
+  double operator[](int) const;
   double &operator[](int);
 
   /** @name Element access. */

Framework/Kernel/inc/MantidKernel/V3D.h

@@ -351,6 +351,7 @@ public:
                     1e-3) const; ///< Determine if there is a master direction
   bool
   nullVector(const double Tol = 1e-3) const; ///< Determine if the point is null
+  bool unitVector(const double tolerance = Kernel::Tolerance) const noexcept;
   bool coLinear(const V3D &, const V3D &) const;
 
   void saveNexus(::NeXus::File *file, const std::string &name) const;

Framework/Kernel/src/Quat.cpp

@@ -213,27 +213,31 @@ void Quat::operator()(const V3D &rX, const V3D &rY, const V3D &rZ) {
   (void)rZ; // Avoid compiler warning
 
   // These are the original axes
-  V3D oX = V3D(1., 0., 0.);
-  V3D oY = V3D(0., 1., 0.);
+  constexpr V3D oX = V3D(1., 0., 0.);
+  constexpr V3D oY = V3D(0., 1., 0.);
 
   // Axis that rotates X
-  V3D ax1 = oX.cross_prod(rX);
-  // Rotation angle from oX to rX
-  double angle1 = oX.angle(rX);
-
+  const V3D ax1 = oX.cross_prod(rX);
   // Create the first quaternion
-  Quat Q1(angle1 * 180.0 / M_PI, ax1);
-
+  Quat Q1;
+  if (!ax1.nullVector()) {
+    // Rotation angle from oX to rX
+    const double angle1 = oX.angle(rX);
+    Q1.setAngleAxis(angle1 * 180.0 / M_PI, ax1);
+  }
   // Now we rotate the original Y using Q1
   V3D roY = oY;
   Q1.rotate(roY);
   // Find the axis that rotates oYr onto rY
-  V3D ax2 = roY.cross_prod(rY);
-  double angle2 = roY.angle(rY);
-  Quat Q2(angle2 * 180.0 / M_PI, ax2);
+  const V3D ax2 = roY.cross_prod(rY);
+  Quat Q2;
+  if (!ax2.nullVector()) {
+    const double angle2 = roY.angle(rY);
+    Q2.setAngleAxis(angle2 * 180.0 / M_PI, ax2);
+  }
 
   // Final = those two rotations in succession; Q1 is done first.
-  Quat final = Q2 * Q1;
+  const Quat final = Q2 * Q1;
 
   // Set it
   this->operator()(final);
@@ -581,7 +585,7 @@ void Quat::setQuat(const Kernel::DblMatrix &rMat) {
  * @param Index :: the index of the value required 0=w, 1=a, 2=b, 3=c
  * @returns a double of the value requested
  */
-const double &Quat::operator[](const int Index) const {
+double Quat::operator[](const int Index) const {
   switch (Index) {
   case 0:
     return w;

Framework/Kernel/src/V3D.cpp

@@ -133,6 +133,9 @@ double V3D::norm() const { return sqrt(x * x + y * y + z * z); }
 */
 double V3D::normalize() {
   const double ND(norm());
+  if (ND == 0) {
+    throw std::runtime_error("Unable to normalize a zero length vector.");
+  }
   this->operator/=(ND);
   return ND;
 }
@@ -254,6 +257,11 @@ bool V3D::nullVector(const double Tol) const
   return true;
 }
 
+bool V3D::unitVector(const double tolerance) const noexcept {
+  const auto l = norm();
+  return std::abs(l - 1.) < tolerance;
+}
+
 int V3D::masterDir(const double Tol) const
 /**
    Calculates the index of the primary direction (if there is one)

Framework/Kernel/test/V3DTest.h

@@ -238,6 +238,10 @@ public:
     TS_ASSERT_EQUALS(b[1], 1.0 / sqrt(3.0));
     TS_ASSERT_EQUALS(b[2], 1.0 / sqrt(3.0));
   }
+  void testNormalizeZeroLengthVectorThrows() {
+    V3D zeroLength;
+    TS_ASSERT_THROWS_ANYTHING(zeroLength.normalize());
+  }
   void testScalarProduct() {
     a(1.0, 2.0, 1.0);
     b(1.0, -2.0, -1.0);
@@ -397,6 +401,20 @@ public:
     TS_ASSERT(a == V3D(-1.0, -2.0, -4.0));
   }
 
+  void test_nullVector() {
+    constexpr V3D null;
+    TS_ASSERT(null.nullVector());
+    constexpr V3D nonNull(0.1, 0., 0.);
+    TS_ASSERT(!nonNull.nullVector())
+  }
+  void test_unitVector() {
+    constexpr V3D null;
+    TS_ASSERT(!null.unitVector())
+    constexpr V3D unit(1.0, 0., 0.);
+    TS_ASSERT(unit.unitVector())
+    constexpr V3D longVector(0.5, -0.5, 0.5);
+    TS_ASSERT(!longVector.unitVector())
+  }
   void test_toString() {
     V3D a(1, 2, 3);
     TS_ASSERT_EQUALS(a.toString(), "1 2 3");
@@ -516,14 +534,6 @@ public:
     TS_ASSERT_DELTA(-9, a8[0], 1.e-2);
     TS_ASSERT_DELTA(1, a8[1], 1.e-2);
     TS_ASSERT_DELTA(-18, a8[2], 1.e-2);
-
-    /*    V3D a9(-3,5,-6);
-     a9.normalize();
-     TS_ASSERT_THROWS_NOTHING(a9.toMillerIndexes(0.1));
-
-     TS_ASSERT_DELTA(-3,a9[0],1.e-3);
-     TS_ASSERT_DELTA( 5,a9[1],1.e-3);
-     TS_ASSERT_DELTA(-6,a9[2],1.e-3);*/
   }
 
   void test_directionAngles_cubic_default() {