Merge pull request #2740 from fmahebert/cleanup_coord_maps

Small code cleanups in coord maps
sxs-collaboration · Jan 9, 2021 · 859e1d9 · 859e1d9
2 parents f0f3afd + 620f024
commit 859e1d9
Show file tree

Hide file tree

Showing 15 changed files with 40 additions and 40 deletions.
diff --git a/src/Domain/BlockLogicalCoordinates.cpp b/src/Domain/BlockLogicalCoordinates.cpp
@@ -48,13 +48,13 @@ std::vector<block_logical_coord_holder<Dim>> block_logical_coordinates(
           const auto moving_inv =
               block.moving_mesh_grid_to_inertial_map().inverse(
                   x_frame, time, functions_of_time);
-          if (not moving_inv) {
+          if (not moving_inv.has_value()) {
             continue;
           }
           // logical to grid map is time-independent.
           const auto inv = block.moving_mesh_logical_to_grid_map().inverse(
               moving_inv.value());
-          if (inv) {
+          if (inv.has_value()) {
             x_logical = inv.value();
           } else {
             continue;  // Not in this block
@@ -73,7 +73,7 @@ std::vector<block_logical_coord_holder<Dim>> block_logical_coordinates(
           // Point is in the grid frame, just map to logical frame.
           const auto inv =
               block.moving_mesh_logical_to_grid_map().inverse(x_frame);
-          if (inv) {
+          if (inv.has_value()) {
             x_logical = inv.value();
           } else {
             continue;  // Not in this block
@@ -82,7 +82,7 @@ std::vector<block_logical_coord_holder<Dim>> block_logical_coordinates(
       } else {  // not block.is_time_dependent()
         if constexpr (std::is_same_v<Frame, ::Frame::Inertial>) {
           const auto inv = block.stationary_map().inverse(x_frame);
-          if (inv) {
+          if (inv.has_value()) {
             x_logical = inv.value();
           } else {
             continue;  // Not in this block
@@ -100,7 +100,7 @@ std::vector<block_logical_coord_holder<Dim>> block_logical_coordinates(
             x_inertial.get(d) = x_frame.get(d);
           }
           const auto inv = block.stationary_map().inverse(x_inertial);
-          if (inv) {
+          if (inv.has_value()) {
             x_logical = inv.value();
           } else {
             continue;  // Not in this block

diff --git a/src/Domain/CoordinateMaps/BulgedCube.cpp b/src/Domain/CoordinateMaps/BulgedCube.cpp
@@ -175,7 +175,7 @@ std::optional<std::array<double, 3>> BulgedCube::inverse(
       // NOLINTNEXTLINE(clang-analyzer-core)
       ::scaling_factor(
       RootFunction{radius_, sphericity_, physical_r_squared, x_sq, y_sq, z_sq});
-  if (not scaling_factor) {
+  if (not scaling_factor.has_value()) {
     return std::nullopt;
   }
   if (use_equiangular_map_) {

diff --git a/src/Domain/CoordinateMaps/CoordinateMap.tpp b/src/Domain/CoordinateMaps/CoordinateMap.tpp
@@ -131,7 +131,7 @@ CoordinateMap<SourceFrame, TargetFrame, Maps...>::inverse_impl(
              std::unique_ptr<domain::FunctionsOfTime::FunctionOfTime>>&
          /*funcs_of_time*/,
          const std::false_type /*is_time_independent*/) noexcept {
-        if (point) {
+        if (point.has_value()) {
           if (LIKELY(not the_map.is_identity())) {
             point = the_map.inverse(point.value());
           }
@@ -144,7 +144,7 @@ CoordinateMap<SourceFrame, TargetFrame, Maps...>::inverse_impl(
              std::unique_ptr<domain::FunctionsOfTime::FunctionOfTime>>&
              funcs_of_time,
          const std::true_type /*is_time_dependent*/) noexcept {
-        if (point) {
+        if (point.has_value()) {
           point = the_map.inverse(point.value(), t, funcs_of_time);
         }
         // this is the inverse function, so the iterator sequence below is

diff --git a/src/Domain/CoordinateMaps/ProductMaps.tpp b/src/Domain/CoordinateMaps/ProductMaps.tpp
@@ -49,7 +49,7 @@ std::optional<std::array<double, Size>> apply_inverse(
       func(std::array<double, sizeof...(Is)>{{coords[Is]...}}, map1);
   auto map2_func = func(
       std::array<double, sizeof...(Js)>{{coords[Map1::dim + Js]...}}, map2);
-  if (map1_func and map2_func) {
+  if (map1_func.has_value() and map2_func.has_value()) {
     return {{{map1_func.value()[Is]..., map2_func.value()[Js]...}}};
   } else {
     return std::nullopt;
@@ -192,7 +192,7 @@ ProductOf3Maps<Map1, Map2, Map3>::inverse(
   auto c1 = map1_.inverse(std::array<double, 1>{{target_coords[0]}});
   auto c2 = map2_.inverse(std::array<double, 1>{{target_coords[1]}});
   auto c3 = map3_.inverse(std::array<double, 1>{{target_coords[2]}});
-  if (c1 and c2 and c3) {
+  if (c1.has_value() and c2.has_value() and c3.has_value()) {
     return {{{c1.value()[0], c2.value()[0], c3.value()[0]}}};
   } else {
     return std::nullopt;

diff --git a/src/Domain/CoordinateMaps/TimeDependent/ProductMaps.tpp b/src/Domain/CoordinateMaps/TimeDependent/ProductMaps.tpp
@@ -63,7 +63,7 @@ std::optional<std::array<double, Size>> apply_inverse(
   auto map2_func = CoordinateMap_detail::apply_inverse_map(
       map2, std::array<double, sizeof...(Js)>{{coords[Map1::dim + Js]...}},
       time, functions_of_time, domain::is_map_time_dependent_t<Map2>{});
-  if (map1_func and map2_func) {
+  if (map1_func.has_value() and map2_func.has_value()) {
     return {{{map1_func.value()[Is]..., map2_func.value()[Js]...}}};
   } else {
     return std::nullopt;
@@ -280,7 +280,7 @@ ProductOf3Maps<Map1, Map2, Map3>::inverse(
   const auto c3 = CoordinateMap_detail::apply_inverse_map(
       map3_, std::array<double, 1>{{target_coords[2]}}, time, functions_of_time,
       domain::is_map_time_dependent_t<Map3>{});
-  if (c1 and c2 and c3) {
+  if (c1.has_value() and c2.has_value() and c3.has_value()) {
     return {{{c1.value()[0], c2.value()[0], c3.value()[0]}}};
   } else {
     return std::nullopt;

diff --git a/tests/Unit/Domain/CoordinateMaps/CMakeLists.txt b/tests/Unit/Domain/CoordinateMaps/CMakeLists.txt
@@ -14,7 +14,6 @@ set(LIBRARY_SOURCES
   Test_Identity.cpp
   Test_ProductMaps.cpp
   Test_Rotation.cpp
-  Test_RotationTimeDep.cpp
   Test_SpecialMobius.cpp
   Test_Tags.cpp
   Test_TimeDependentHelpers.cpp

diff --git a/tests/Unit/Domain/CoordinateMaps/Test_BulgedCube.cpp b/tests/Unit/Domain/CoordinateMaps/Test_BulgedCube.cpp
@@ -36,14 +36,14 @@ void test_bulged_cube_fail() {
   const std::array<double, 3> test_mapped_point4{{2.0, 2.0, 2.0}};
   const std::array<double, 3> test_mapped_point5{{3.0, 0.0, 0.0}};
 
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point1)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point2)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point3)));
-  if (map.inverse(test_mapped_point4)) {
+  CHECK_FALSE(map.inverse(test_mapped_point1).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point2).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point3).has_value());
+  if (map.inverse(test_mapped_point4).has_value()) {
     CHECK_ITERABLE_APPROX(map(map.inverse(test_mapped_point4).value()),
                           test_mapped_point4);
   }
-  if (map.inverse(test_mapped_point5)) {
+  if (map.inverse(test_mapped_point5).has_value()) {
     CHECK_ITERABLE_APPROX(map(map.inverse(test_mapped_point5).value()),
                           test_mapped_point5);
   }

diff --git a/tests/Unit/Domain/CoordinateMaps/Test_Frustum.cpp b/tests/Unit/Domain/CoordinateMaps/Test_Frustum.cpp
@@ -73,10 +73,10 @@ void test_frustum_fail() noexcept {
   // this point) or it should return nullopt.
   const std::array<double, 3> test_mapped_point4{{0.0, 0.0, 9.0}};
 
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point1)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point2)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point3)));
-  if (map.inverse(test_mapped_point4)) {
+  CHECK_FALSE(map.inverse(test_mapped_point1).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point2).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point3).has_value());
+  if (map.inverse(test_mapped_point4).has_value()) {
     CHECK_ITERABLE_APPROX(map(map.inverse(test_mapped_point4).value()),
                           test_mapped_point4);
   }

diff --git a/tests/Unit/Domain/CoordinateMaps/Test_ProductMaps.cpp b/tests/Unit/Domain/CoordinateMaps/Test_ProductMaps.cpp
@@ -39,8 +39,8 @@ void test_product_two_maps_fail() {
     // Should be ok
     const std::array<double, 3> mapped_point2{{3.5, 3.0, -1.0}};
 
-    CHECK_FALSE(static_cast<bool>(map.inverse(mapped_point1)));
-    CHECK(static_cast<bool>(map.inverse(mapped_point2)));
+    CHECK_FALSE(map.inverse(mapped_point1).has_value());
+    CHECK(map.inverse(mapped_point2).has_value());
     CHECK_ITERABLE_APPROX(map(map.inverse(mapped_point2).value()),
                           mapped_point2);
   }
@@ -55,8 +55,8 @@ void test_product_two_maps_fail() {
     // Should be ok
     const std::array<double, 3> mapped_point2{{3.0, -1.0, 3.5}};
 
-    CHECK_FALSE(static_cast<bool>(map.inverse(mapped_point1)));
-    CHECK(static_cast<bool>(map.inverse(mapped_point2)));
+    CHECK_FALSE(map.inverse(mapped_point1).has_value());
+    CHECK(map.inverse(mapped_point2).has_value());
     CHECK_ITERABLE_APPROX(map(map.inverse(mapped_point2).value()),
                           mapped_point2);
   }

diff --git a/tests/Unit/Domain/CoordinateMaps/Test_SpecialMobius.cpp b/tests/Unit/Domain/CoordinateMaps/Test_SpecialMobius.cpp
@@ -85,7 +85,7 @@ void test_map() {
   // Point at which the map is singular.
   // Since |mu|<1, this point also has x outside [-1,1].
   const std::array<double, 3> bad_point{{-1.0 / mu, 0.0, 0.0}};
-  CHECK_FALSE(static_cast<bool>(special_mobius_map.inverse(bad_point)));
+  CHECK_FALSE(special_mobius_map.inverse(bad_point).has_value());
 }
 
 void test_large_mu() {

diff --git a/tests/Unit/Domain/CoordinateMaps/Test_Wedge2D.cpp b/tests/Unit/Domain/CoordinateMaps/Test_Wedge2D.cpp
@@ -141,10 +141,10 @@ void test_wedge2d_fail() noexcept {
   // this point) or it should return nullopt.
   const std::array<double, 2> test_mapped_point4{{100.0, -6.0}};
 
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point1)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point2)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point3)));
-  if (map.inverse(test_mapped_point4)) {
+  CHECK_FALSE(map.inverse(test_mapped_point1).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point2).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point3).has_value());
+  if (map.inverse(test_mapped_point4).has_value()) {
     CHECK_ITERABLE_APPROX(map(map.inverse(test_mapped_point4).value()),
                           test_mapped_point4);
   }

diff --git a/tests/Unit/Domain/CoordinateMaps/Test_Wedge3D.cpp b/tests/Unit/Domain/CoordinateMaps/Test_Wedge3D.cpp
@@ -288,17 +288,17 @@ void test_wedge3d_fail() noexcept {
   const std::array<double, 3> test_mapped_point6{{30.0, sqrt(298.0), 1.0}};
   const std::array<double, 3> test_mapped_point7{{2.0, 4.0, 6.0}};
 
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point1)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point2)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point3)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point4)));
-  CHECK_FALSE(static_cast<bool>(map.inverse(test_mapped_point5)));
-  if (map.inverse(test_mapped_point6)) {
+  CHECK_FALSE(map.inverse(test_mapped_point1).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point2).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point3).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point4).has_value());
+  CHECK_FALSE(map.inverse(test_mapped_point5).has_value());
+  if (map.inverse(test_mapped_point6).has_value()) {
     Approx my_approx = Approx::custom().epsilon(1.e-10).scale(1.0);
     CHECK_ITERABLE_CUSTOM_APPROX(map(map.inverse(test_mapped_point6).value()),
                                  test_mapped_point6, my_approx);
   }
-  if (map.inverse(test_mapped_point7)) {
+  if (map.inverse(test_mapped_point7).has_value()) {
     CHECK_ITERABLE_APPROX(map(map.inverse(test_mapped_point7).value()),
                           test_mapped_point7);
   }

diff --git a/tests/Unit/Domain/CoordinateMaps/TimeDependent/CMakeLists.txt b/tests/Unit/Domain/CoordinateMaps/TimeDependent/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LIBRARY "Test_CoordinateMapsTimeDependent")
 set(LIBRARY_SOURCES
   Test_CubicScale.cpp
   Test_ProductMaps.cpp
+  Test_Rotation.cpp
   Test_Translation.cpp
   )
 

diff --git a/tests/Unit/Domain/CoordinateMaps/TimeDependent/Test_CubicScale.cpp b/tests/Unit/Domain/CoordinateMaps/TimeDependent/Test_CubicScale.cpp
@@ -100,7 +100,7 @@ void test_boundaries() {
 
       CHECK_ITERABLE_APPROX(scale_map(point_xi, t, f_of_t_list), mapped_point);
       REQUIRE(
-          static_cast<bool>(scale_map.inverse(mapped_point, t, f_of_t_list)));
+          scale_map.inverse(mapped_point, t, f_of_t_list).has_value());
       CHECK_ITERABLE_APPROX(
           scale_map.inverse(mapped_point, t, f_of_t_list).value(), point_xi);
       t += dt;

diff --git a/...n/CoordinateMaps/Test_RotationTimeDep.cpp → ...inateMaps/TimeDependent/Test_Rotation.cpp b/...n/CoordinateMaps/Test_RotationTimeDep.cpp → ...inateMaps/TimeDependent/Test_Rotation.cpp
@@ -49,7 +49,7 @@ std::array<double, 2> expected_frame_velocity(
 }  // namespace
 
 namespace domain {
-SPECTRE_TEST_CASE("Unit.Domain.CoordinateMaps.RotationTimeDep",
+SPECTRE_TEST_CASE("Unit.Domain.CoordinateMaps.TimeDependent.Rotation",
                   "[Domain][Unit]") {
   double t{-1.0};
   const double dt{0.6};