adressing MR comments

Core/include/Acts/Surfaces/CylinderBounds.hpp

@@ -140,13 +140,13 @@ inline bool CylinderBounds::coversFullAzimuth() const {
 }
 
 inline void CylinderBounds::checkConsistency() noexcept(false) {
-  if (get(eR) < 0.) {
+  if (get(eR) <= 0.) {
     throw std::invalid_argument("CylinderBounds: invalid radial setup.");
   }
-  if (get(eHalfLengthZ) < 0.) {
+  if (get(eHalfLengthZ) <= 0.) {
     throw std::invalid_argument("CylinderBounds: invalid length setup.");
   }
-  if (get(eHalfPhiSector) < 0. or get(eHalfPhiSector) > M_PI) {
+  if (get(eHalfPhiSector) <= 0. or get(eHalfPhiSector) > M_PI) {
     throw std::invalid_argument("CylinderBounds: invalid phi sector setup.");
   }
   if (get(eAveragePhi) != detail::radian_sym(get(eAveragePhi))) {

Core/include/Acts/Surfaces/DiamondBounds.hpp

@@ -126,9 +126,8 @@ inline std::vector<double> DiamondBounds::values() const {
 
 inline void DiamondBounds::checkConsistency() noexcept(false) {
   if (std::any_of(m_values.begin(), m_values.end(),
-                  [](auto v) { return v < 0.; })) {
-    throw std::invalid_argument(
-        "DiamondBounds: negative half length provided.");
+                  [](auto v) { return v <= 0.; })) {
+    throw std::invalid_argument("DiamondBounds: negative half length.");
   }
   if (get(eHalfLengthXnegY) > get(eHalfLengthXzeroY) or
       get(eHalfLengthXposY) > get(eHalfLengthXzeroY)) {

Core/include/Acts/Surfaces/DiscTrapezoidBounds.hpp

@@ -217,10 +217,10 @@ inline std::vector<double> DiscTrapezoidBounds::values() const {
 }
 
 inline void DiscTrapezoidBounds::checkConsistency() noexcept(false) {
-  if (get(eMinR) * get(eMaxR) < 0. or get(eMinR) > get(eMaxR)) {
+  if (get(eMinR) < 0. or get(eMaxR) <= 0. or get(eMinR) > get(eMaxR)) {
     throw std::invalid_argument("DiscTrapezoidBounds: invalid radial setup.");
   }
-  if (get(eHalfLengthXminR) * get(eHalfLengthXmaxR) < 0.) {
+  if (get(eHalfLengthXminR) < 0. or get(eHalfLengthXmaxR) <= 0.) {
     throw std::invalid_argument("DiscTrapezoidBounds: negative length given.");
   }
   if (get(eAveragePhi) != detail::radian_sym(get(eAveragePhi))) {

Core/include/Acts/Surfaces/EllipseBounds.hpp

@@ -129,10 +129,10 @@ inline std::vector<double> EllipseBounds::values() const {
 }
 
 inline void EllipseBounds::checkConsistency() noexcept(false) {
-  if (get(eMinR0) * get(eMaxR0) < 0. or get(eMinR0) > get(eMaxR0)) {
+  if (get(eMinR0) <= 0. or get(eMaxR0) <= 0. or get(eMinR0) > get(eMaxR0)) {
     throw std::invalid_argument("EllipseBounds: invalid first coorindate.");
   }
-  if (get(eMinR1) * get(eMaxR1) < 0. or get(eMinR1) > get(eMaxR1)) {
+  if (get(eMinR1) <= 0. or get(eMaxR1) <= 0. or get(eMinR1) > get(eMaxR1)) {
     throw std::invalid_argument("EllipseBounds: invalid second coorindate.");
   }
   if (get(eHalfPhiSector) < 0. or get(eHalfPhiSector) > M_PI) {

Core/include/Acts/Surfaces/LineBounds.hpp

@@ -94,8 +94,11 @@ inline std::vector<double> LineBounds::values() const {
 }
 
 inline void LineBounds::checkConsistency() noexcept(false) {
-  if (get(eR) * get(eHalfLengthZ) <= 0.) {
-    throw std::invalid_argument("LineBounds: zero/negative radius/legnth.");
+  if (get(eR) < 0.) {
+    throw std::invalid_argument("LineBounds: zero radius.");
+  }
+  if (get(eHalfLengthZ) <= 0.) {
+    throw std::invalid_argument("LineBounds: zero/negative length.");
   }
 }
 

Core/include/Acts/Surfaces/RadialBounds.hpp

@@ -168,7 +168,7 @@ inline std::vector<double> RadialBounds::values() const {
 }
 
 inline void RadialBounds::checkConsistency() noexcept(false) {
-  if (get(eMinR) * get(eMaxR) < 0. or get(eMinR) > get(eMaxR)) {
+  if (get(eMinR) < 0. or get(eMaxR) <= 0. or get(eMinR) > get(eMaxR)) {
     throw std::invalid_argument("RadialBounds: invalid radial setup");
   }
   if (get(eHalfPhiSector) < 0. or get(eHalfPhiSector) > M_PI) {

Core/include/Acts/Surfaces/TrapezoidBounds.hpp

@@ -157,7 +157,7 @@ inline std::vector<double> TrapezoidBounds::values() const {
 }
 
 inline void TrapezoidBounds::checkConsistency() noexcept(false) {
-  if (get(eHalfLengthXnegY) * get(eHalfLengthXposY) <= 0.) {
+  if (get(eHalfLengthXnegY) <= 0. or get(eHalfLengthXposY) <= 0.) {
     throw std::invalid_argument("TrapezoidBounds: invalid local x setup");
   }
   if (get(eHalfLengthY) <= 0.) {

Tests/UnitTests/Core/Surfaces/DiscTrapezoidBoundsTests.cpp

@@ -20,6 +20,7 @@ namespace Acts {
 
 namespace Test {
 BOOST_AUTO_TEST_SUITE(Surfaces)
+
 /// Unit tests for DiscTrapezoidBounds constrcuctors
 BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsConstruction) {
   double minHalfX(1.0), maxHalfX(5.0), rMin(2.0), rMax(6.0), averagePhi(0.0),
@@ -45,6 +46,61 @@ BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsConstruction) {
   BOOST_CHECK_EQUAL(copied.type(), SurfaceBounds::eDiscTrapezoid);
 }
 
+// Streaning and recreation test
+BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsRecreation) {
+  double minHalfX(1.0), maxHalfX(5.0), rMin(2.0), rMax(6.0), averagePhi(0.0),
+      stereo(0.1);
+
+  DiscTrapezoidBounds original(minHalfX, maxHalfX, rMin, rMax, averagePhi,
+                               stereo);
+  auto valvector = original.values();
+  std::array<double, DiscTrapezoidBounds::eSize> values;
+  std::copy_n(valvector.begin(), DiscTrapezoidBounds::eSize, values.begin());
+  DiscTrapezoidBounds recreated(values);
+  BOOST_CHECK_EQUAL(recreated, original);
+}
+
+// Unit tests for AnnulusBounds exception throwing
+BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsExceptions) {
+  double minHalfX(1.0), maxHalfX(5.0), rMin(2.0), rMax(6.0), averagePhi(0.0),
+      stereo(0.1);
+
+  // Exception for opening neg min half x < 0
+  BOOST_CHECK_THROW(
+      DiscTrapezoidBounds(-minHalfX, maxHalfX, rMin, rMax, averagePhi, stereo),
+      std::logic_error);
+
+  // Exception for opening neg max half x < 0
+  BOOST_CHECK_THROW(
+      DiscTrapezoidBounds(minHalfX, -maxHalfX, rMin, rMax, averagePhi, stereo),
+      std::logic_error);
+
+  // Exception for opening neg min and max half x < 0
+  BOOST_CHECK_THROW(
+      DiscTrapezoidBounds(-minHalfX, -maxHalfX, rMin, rMax, averagePhi, stereo),
+      std::logic_error);
+
+  // Exception for opening neg r min
+  BOOST_CHECK_THROW(
+      DiscTrapezoidBounds(minHalfX, maxHalfX, -rMin, rMax, averagePhi, stereo),
+      std::logic_error);
+
+  // Exception for opening neg r max
+  BOOST_CHECK_THROW(
+      DiscTrapezoidBounds(minHalfX, maxHalfX, rMin, -rMax, averagePhi, stereo),
+      std::logic_error);
+
+  // Exception for opening neg r min and r max
+  BOOST_CHECK_THROW(
+      DiscTrapezoidBounds(minHalfX, maxHalfX, -rMin, -rMax, averagePhi, stereo),
+      std::logic_error);
+
+  // Exception for out of bound average phi
+  BOOST_CHECK_THROW(
+      DiscTrapezoidBounds(minHalfX, maxHalfX, rMin, rMax, 4., stereo),
+      std::logic_error);
+}
+
 /// Unit tests for DiscTrapezoidBounds properties
 BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsProperties) {
   double minHalfX(1.0), maxHalfX(5.0), rMin(2.0), rMax(6.0),

Tests/UnitTests/Core/Surfaces/DiscTrapezoidalBoundsTests.cpp

-// This file is part of the Acts project.
-//
-// Copyright (C) 2017-2018 CERN for the benefit of the Acts project
-//
-// This Source Code Form is subject to the terms of the Mozilla Public
-// License, v. 2.0. If a copy of the MPL was not distributed with this
-// file, You can obtain one at http://mozilla.org/MPL/2.0/.
-
-#include <boost/test/data/test_case.hpp>
-#include <boost/test/tools/output_test_stream.hpp>
-#include <boost/test/unit_test.hpp>
-
-#include <limits>
-
-#include "Acts/Surfaces/DiscTrapezoidBounds.hpp"
-#include "Acts/Tests/CommonHelpers/FloatComparisons.hpp"
-#include "Acts/Utilities/Definitions.hpp"
-
-namespace Acts {
-
-namespace Test {
-BOOST_AUTO_TEST_SUITE(Surfaces)
-/// Unit tests for DiscTrapezoidBounds constrcuctors
-BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsConstruction) {
-  double minHalfX(1.0), maxHalfX(5.0), rMin(2.0), rMax(6.0), averagePhi(0.0),
-      stereo(0.1);
-  // test default construction
-  // DiscTrapezoidBounds defaultConstructedDiscTrapezoidBounds;  should be
-  // deleted
-  //
-  /// Test construction with dimensions and default stereo
-  BOOST_CHECK_EQUAL(
-      DiscTrapezoidBounds(minHalfX, maxHalfX, rMin, rMax, averagePhi).type(),
-      SurfaceBounds::eDiscTrapezoid);
-  //
-  /// Test construction with all dimensions
-  BOOST_CHECK_EQUAL(
-      DiscTrapezoidBounds(minHalfX, maxHalfX, rMin, rMax, averagePhi, stereo)
-          .type(),
-      SurfaceBounds::eDiscTrapezoid);
-  //
-  /// Copy constructor
-  DiscTrapezoidBounds original(minHalfX, maxHalfX, rMin, rMax, averagePhi);
-  DiscTrapezoidBounds copied(original);
-  BOOST_CHECK_EQUAL(copied.type(), SurfaceBounds::eDiscTrapezoid);
-}
-
-/// Unit tests for DiscTrapezoidBounds properties
-BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsProperties) {
-  double minHalfX(1.0), maxHalfX(5.0), rMin(2.0), rMax(6.0),
-      averagePhi(0.0) /*, stereo(0.1)*/;
-  /// Test clone
-  DiscTrapezoidBounds discTrapezoidBoundsObject(minHalfX, maxHalfX, rMin, rMax,
-                                                averagePhi);
-  auto pClonedDiscTrapezoidBounds = discTrapezoidBoundsObject.clone();
-  BOOST_CHECK_NE(pClonedDiscTrapezoidBounds, nullptr);
-  delete pClonedDiscTrapezoidBounds;
-  //
-  /// Test type() (redundant; already used in constructor confirmation)
-  BOOST_CHECK_EQUAL(discTrapezoidBoundsObject.type(),
-                    SurfaceBounds::eDiscTrapezoid);
-  //
-  /// Test distanceToBoundary
-  Vector2D origin(0., 0.);
-  Vector2D outside(30., 0.);
-  Vector2D inSurface(2., 0.0);
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.distanceToBoundary(origin), 2.0,
-                  1e-6);
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.distanceToBoundary(outside), 24.0,
-                  1e-6);
-  //
-  /// Test dump
-  boost::test_tools::output_test_stream dumpOuput;
-  discTrapezoidBoundsObject.toStream(dumpOuput);
-  BOOST_CHECK(dumpOuput.is_equal(
-      "Acts::DiscTrapezoidBounds: (innerRadius, outerRadius, halfLengthXminR, "
-      "halfLengthXmaxR, halfLengthY, halfPhiSector, averagePhi, rCenter, "
-      "stereo) = "
-      "(2.0000000, 6.0000000, 1.0000000, 5.0000000, 0.7922870, 0.9851108, "
-      "0.0000000, 2.5243378, 0.0000000)"));
-  //
-  /// Test inside
-  BOOST_CHECK(discTrapezoidBoundsObject.inside(inSurface, BoundaryCheck(true)));
-  BOOST_CHECK(!discTrapezoidBoundsObject.inside(outside, BoundaryCheck(true)));
-  //
-  /// Test rMin
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.rMin(), rMin, 1e-6);
-  //
-  /// Test rMax
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.rMax(), rMax, 1e-6);
-  //
-  /// Test averagePhi
-  CHECK_SMALL(discTrapezoidBoundsObject.averagePhi(), 1e-9);
-  //
-  /// Test rCenter (redundant; not configurable)
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.rCenter(), 2.524337798, 1e-6);
-  //
-  /// Test halfPhiSector (redundant; not configurable)
-  CHECK_SMALL(discTrapezoidBoundsObject.stereo(), 1e-6);
-  //
-  /// Test minHalflengthX
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.minHalflengthX(), minHalfX, 1e-6);
-  //
-  /// Test maxHalflengthX
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.maxHalflengthX(), maxHalfX, 1e-6);
-  //
-  /// Test halflengthY
-  CHECK_CLOSE_REL(discTrapezoidBoundsObject.halflengthY(), 0.792286991, 1e-6);
-}
-/// Unit test for testing DiscTrapezoidBounds assignment
-BOOST_AUTO_TEST_CASE(DiscTrapezoidBoundsAssignment) {
-  double minHalfX(1.0), maxHalfX(5.0), rMin(2.0), rMax(6.0), averagePhi(0.0),
-      stereo(0.1);
-  DiscTrapezoidBounds discTrapezoidBoundsObject(minHalfX, maxHalfX, rMin, rMax,
-                                                averagePhi, stereo);
-  // operator == not implemented in this class
-  //
-  /// Test assignment
-  DiscTrapezoidBounds assignedDiscTrapezoidBoundsObject(2.1, 6.6, 3.4, 4.2,
-                                                        33.);
-  assignedDiscTrapezoidBoundsObject = discTrapezoidBoundsObject;
-  BOOST_CHECK_EQUAL(assignedDiscTrapezoidBoundsObject,
-                    discTrapezoidBoundsObject);
-}
-
-BOOST_AUTO_TEST_SUITE_END()
-
-}  // namespace Test
-
-}  // namespace Acts

Tests/UnitTests/Core/Surfaces/EllipseBoundsTests.cpp

@@ -67,6 +67,10 @@ BOOST_AUTO_TEST_CASE(ConeBoundsExceptions) {
   BOOST_CHECK_THROW(
       EllipseBounds(minRad0, -maxRad0, minRad1, maxRad1, phiSector, averagePhi),
       std::logic_error);
+  // Exception for opening minR0 and maxR0 < 0
+  BOOST_CHECK_THROW(EllipseBounds(-minRad0, -maxRad0, minRad1, maxRad1,
+                                  phiSector, averagePhi),
+                    std::logic_error);
   // Exception for swapped minR0/maxR0
   BOOST_CHECK_THROW(
       EllipseBounds(maxRad0, minRad0, minRad1, maxRad1, phiSector, averagePhi),
@@ -79,6 +83,10 @@ BOOST_AUTO_TEST_CASE(ConeBoundsExceptions) {
   BOOST_CHECK_THROW(
       EllipseBounds(minRad0, maxRad0, minRad1, -maxRad1, phiSector, averagePhi),
       std::logic_error);
+  // Exception for opening maxR1 < 0
+  BOOST_CHECK_THROW(EllipseBounds(minRad0, maxRad0, -minRad1, -maxRad1,
+                                  phiSector, averagePhi),
+                    std::logic_error);
   // Exception for swapped minR1/maxR1
   BOOST_CHECK_THROW(
       EllipseBounds(minRad0, maxRad0, maxRad1, minRad1, phiSector, averagePhi),

Tests/UnitTests/Core/Surfaces/LineBoundsTests.cpp

@@ -51,6 +51,10 @@ BOOST_AUTO_TEST_CASE(LineBoundsExceptions) {
   // Negative half length
   BOOST_CHECK_THROW(LineBounds(nominalRadius, -nominalHalfLength),
                     std::logic_error);
+
+  // Negative radius and half length
+  BOOST_CHECK_THROW(LineBounds(-nominalRadius, -nominalHalfLength),
+                    std::logic_error);
 }
 
 /// Unit test for testing LineBounds assignment

Tests/UnitTests/Core/Surfaces/RadialBoundsTests.cpp

@@ -65,6 +65,10 @@ BOOST_AUTO_TEST_CASE(RadialBoundsException) {
   BOOST_CHECK_THROW(RadialBounds(minRadius, -maxRadius, halfPhiSector, avgPhi),
                     std::logic_error);
 
+  // Negative inner and outer radius
+  BOOST_CHECK_THROW(RadialBounds(-minRadius, -maxRadius, halfPhiSector, avgPhi),
+                    std::logic_error);
+
   // Swapped radii
   BOOST_CHECK_THROW(RadialBounds(maxRadius, minRadius, halfPhiSector, avgPhi),
                     std::logic_error);

Tests/UnitTests/Core/Surfaces/TrapezoidBoundsTests.cpp

@@ -64,6 +64,10 @@ BOOST_AUTO_TEST_CASE(TrapezoidBoundsException) {
   BOOST_CHECK_THROW(TrapezoidBounds(minHalfX, -maxHalfX, halfY),
                     std::logic_error);
 
+  // Negative x at miny and max y
+  BOOST_CHECK_THROW(TrapezoidBounds(-minHalfX, -maxHalfX, halfY),
+                    std::logic_error);
+
   // Negative y
   BOOST_CHECK_THROW(TrapezoidBounds(minHalfX, maxHalfX, -halfY),
                     std::logic_error);