Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor cleanup of testing framework code #5622

Merged
merged 3 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Utilities/System/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ spectre_target_sources(
${LIBRARY}
PRIVATE
Abort.cpp
Exit.cpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you intend to actually make the dependence on Charm PRIVATE in this file as the commit message promises? If so I think it would be good to also add a comment there why you made it private so it won't accidentally be made public again later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, but I couldn't get it to work because we still have some cyclic library dependencies. I'll remove the comment from the commit message

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you understand the cyclic dependency? How can making a dependency private introduce a cyclic dependency?

ParallelInfo.cpp
Prefetch.cpp
)
Expand Down
18 changes: 18 additions & 0 deletions src/Utilities/System/Exit.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Distributed under the MIT License.
// See LICENSE.txt for details.

#include "Utilities/System/Exit.hpp"

#include <charm++.h>
#include <exception>

namespace sys {

[[noreturn]] void exit(const int exit_code) {
CkExit(exit_code);
// the following call is never reached, but suppresses the warning that
// a 'noreturn' function does return
std::terminate(); // LCOV_EXCL_LINE
}

} // namespace sys
10 changes: 1 addition & 9 deletions src/Utilities/System/Exit.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,11 @@

#pragma once

#include <charm++.h>
#include <exception>

namespace sys {

/// \ingroup UtilitiesGroup
/// \brief Exit the program normally.
/// This should only be called once over all processors.
[[noreturn]] inline void exit(const int exit_code = 0) {
CkExit(exit_code);
// the following call is never reached, but suppresses the warning that
// a 'noreturn' function does return
std::terminate(); // LCOV_EXCL_LINE
}
[[noreturn]] void exit(int exit_code = 0);

} // namespace sys
28 changes: 28 additions & 0 deletions src/Utilities/System/ParallelInfo.cpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,40 @@
// Distributed under the MIT License.
// See LICENSE.txt for details.

#include <charm++.h>
#include <iomanip>
#include <sstream>

#include "Utilities/System/ParallelInfo.hpp"

namespace sys {
int number_of_procs() { return CkNumPes(); }

int my_proc() { return CkMyPe(); }

int number_of_nodes() { return CkNumNodes(); }

int my_node() { return CkMyNode(); }

int procs_on_node([[maybe_unused]] const int node_index) {
return CkNodeSize(node_index);
}

int my_local_rank() { return CkMyRank(); }

int first_proc_on_node([[maybe_unused]] const int node_index) {
return CkNodeFirst(node_index);
}

int node_of([[maybe_unused]] const int proc_index) {
return CkNodeOf(proc_index);
}

int local_rank_of([[maybe_unused]] const int proc_index) {
return CkRankOf(proc_index);
}

double wall_time() { return CkWallTimer(); }

std::string pretty_wall_time(const double total_seconds) {
// Subseconds don't really matter so just ignore them. This gives nice round
Expand Down
41 changes: 10 additions & 31 deletions src/Utilities/System/ParallelInfo.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,90 +9,69 @@

#pragma once

#include <charm++.h>
#include <string>

namespace sys {
/*!
* \ingroup UtilitiesGroup
* \brief Number of processing elements.
*/
inline int number_of_procs() { return CkNumPes(); }
int number_of_procs();

/*!
* \ingroup UtilitiesGroup
* \brief %Index of my processing element.
*/
inline int my_proc() { return CkMyPe(); }
int my_proc();

/*!
* \ingroup UtilitiesGroup
* \brief Number of nodes.
*/
inline int number_of_nodes() { return CkNumNodes(); }
int number_of_nodes();

/*!
* \ingroup UtilitiesGroup
* \brief %Index of my node.
*/
inline int my_node() { return CkMyNode(); }
int my_node();

/*!
* \ingroup UtilitiesGroup
* \brief Number of processing elements on the given node.
*/
inline int procs_on_node(const int node_index) {
// When using the verbs-linux-x86_64 non-SMP build of Charm++ these
// functions have unused-parameter warnings. This is remedied by
// casting the integer to a void which results in no extra assembly
// code being generated. We use this instead of pragmas because we
// would require one pragma for GCC and one for clang, which would
// result in code duplication. Commenting out the variable
// nodeIndex gives compilation failures on most Charm++ builds since
// they actually use the variable. Charm++ plz...
static_cast<void>(node_index);
return CkNodeSize(node_index);
}
int procs_on_node(int node_index);

/*!
* \ingroup UtilitiesGroup
* \brief The local index of my processing element on my node.
* This is in the interval 0, ..., procs_on_node(my_node()) - 1.
*/
inline int my_local_rank() { return CkMyRank(); }
int my_local_rank();

/*!
* \ingroup UtilitiesGroup
* \brief %Index of first processing element on the given node.
*/
inline int first_proc_on_node(const int node_index) {
static_cast<void>(node_index);
return CkNodeFirst(node_index);
}
int first_proc_on_node(int node_index);

/*!
* \ingroup UtilitiesGroup
* \brief %Index of the node for the given processing element.
*/
inline int node_of(const int proc_index) {
static_cast<void>(proc_index);
return CkNodeOf(proc_index);
}
int node_of(int proc_index);

/*!
* \ingroup UtilitiesGroup
* \brief The local index for the given processing element on its node.
*/
inline int local_rank_of(const int proc_index) {
static_cast<void>(proc_index);
return CkRankOf(proc_index);
}
int local_rank_of(int proc_index);

/*!
* \ingroup UtilitiesGroup
* \brief The elapsed wall time in seconds.
*/
inline double wall_time() { return CkWallTimer(); }
double wall_time();

/// @{
/// \ingroup UtilitiesGroup
Expand Down
4 changes: 2 additions & 2 deletions tests/Unit/DataStructures/Test_ApplyMatrices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ struct CheckApply<LocalScalarTag, LocalTensorTag, Dim, Dim> {
// factor of 6 or so.
Approx custom_approx = Approx::custom().epsilon(1.e-13).scale(1.0);
for (const auto p : result - expected) {
CHECK_COMPLEX_CUSTOM_APPROX(p, 0.0, custom_approx);
CHECK_ITERABLE_CUSTOM_APPROX(p, 0.0, custom_approx);
}
const auto ref_matrices =
make_array<std::reference_wrapper<const Matrix>, Dim>(matrices);
Expand All @@ -126,7 +126,7 @@ struct CheckApply<LocalScalarTag, LocalTensorTag, Dim, Dim> {
const auto vector_result = apply_matrices(
matrices, get(get<LocalScalarTag>(source_data)), source_mesh.extents());
for (const auto p : vector_result - get(get<LocalScalarTag>(expected))) {
CHECK_COMPLEX_APPROX(p, 0.0);
CHECK_ITERABLE_APPROX(p, 0.0);
}
CHECK(apply_matrices(ref_matrices, get(get<LocalScalarTag>(source_data)),
source_mesh.extents()) == vector_result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ void check_rotation_matrix_helpers_3d(
Approx custom_approx = Approx::custom().epsilon(5e-12).scale(1.0);
{
INFO("Check rotation matrix");
CHECK_MATRIX_CUSTOM_APPROX(rot_matrix, expected_rot_matrix, custom_approx);
CHECK_ITERABLE_CUSTOM_APPROX(rot_matrix, expected_rot_matrix,
custom_approx);
}
{
INFO("Check rotation matrix deriv");
CHECK_MATRIX_CUSTOM_APPROX(rot_matrix_deriv, expected_rot_matrix_deriv,
custom_approx);
CHECK_ITERABLE_CUSTOM_APPROX(rot_matrix_deriv, expected_rot_matrix_deriv,
custom_approx);
}
}

Expand All @@ -57,11 +58,11 @@ void check_rotation_matrix_helpers_2d(const double init_omega,

{
INFO("Check rotation matrix");
CHECK_MATRIX_APPROX(rot_matrix, expected_rot_matrix);
CHECK_ITERABLE_APPROX(rot_matrix, expected_rot_matrix);
}
{
INFO("Check rotation matrix deriv");
CHECK_MATRIX_APPROX(rot_matrix_deriv, expected_rot_matrix_deriv);
CHECK_ITERABLE_APPROX(rot_matrix_deriv, expected_rot_matrix_deriv);
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Unit/Domain/Creators/Test_ExpandOverBlocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ SPECTRE_TEST_CASE("Unit.Domain.ExpandOverBlocks", "[Domain][Unit]") {
std::vector<std::array<size_t, 3>>{{2, 3, 4}, {3, 4, 5}, {4, 5, 6}}, 3,
{{2, 3, 4}, {3, 4, 5}, {4, 5, 6}});
CHECK_THROWS_WITH(
SINGLE_ARG(ExpandOverBlocks<size_t, 1>{3}(
(ExpandOverBlocks<size_t, 1>{3}(
std::vector<std::array<size_t, 1>>{{2}, {3}})),
Catch::Matchers::ContainsSubstring(
"You supplied 2 values, but the domain creator has 3 blocks."));
Expand Down
3 changes: 2 additions & 1 deletion tests/Unit/Evolution/DgSubcell/Test_SliceData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "Framework/TestingFramework.hpp"

#include <array>
#include <cmath>
#include <cstddef>
#include <numeric>
#include <optional>
Expand Down Expand Up @@ -115,7 +116,7 @@ void test_slice_data(
disable_floating_point_exceptions();
for (const auto& [_, data_in_direction] : sliced_data) {
for (const double& at_point : data_in_direction) {
CHECK(isnan(at_point));
CHECK(std::isnan(at_point));
}
}
enable_floating_point_exceptions();
Expand Down
3 changes: 2 additions & 1 deletion tests/Unit/Evolution/DgSubcell/Test_SliceTensor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "Framework/TestingFramework.hpp"

#include <cmath>
#include <cstddef>
#include <numeric>

Expand Down Expand Up @@ -77,7 +78,7 @@ void test() {
CHECK(get(sliced_scalar).size() ==
get(expected_sliced_scalar).size());
for (const double value_at_point : get(sliced_scalar)) {
CHECK(isnan(value_at_point));
CHECK(std::isnan(value_at_point));
}
enable_floating_point_exceptions();
#endif
Expand Down
3 changes: 2 additions & 1 deletion tests/Unit/Evolution/DgSubcell/Test_SliceVariable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "Framework/TestingFramework.hpp"

#include <cmath>
#include <cstddef>
#include <numeric>

Expand Down Expand Up @@ -56,7 +57,7 @@ void test_slice(
disable_floating_point_exceptions();
CHECK(sliced_vars.size() == expected_sliced_vars.size());
for (size_t i = 0; i < sliced_vars.size(); ++i) {
CHECK(isnan(
CHECK(std::isnan(
*std::next(sliced_vars.data(), static_cast<std::ptrdiff_t>(i))));
}
CHECK(sliced_vars != expected_sliced_vars);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ void check_22_and_33_modes(
const std::complex<double>& expected_22_mode,
const std::complex<double>& expected_33_mode, const size_t l_max,
Approx bondi_approx) {
CHECK_COMPLEX_CUSTOM_APPROX(
CHECK_ITERABLE_CUSTOM_APPROX(
goldberg_modes.data()[Spectral::Swsh::goldberg_mode_index(l_max, 2, 2)],
expected_22_mode, bondi_approx);
CHECK_COMPLEX_CUSTOM_APPROX(
CHECK_ITERABLE_CUSTOM_APPROX(
goldberg_modes.data()[Spectral::Swsh::goldberg_mode_index(l_max, 2, -2)],
expected_22_mode, bondi_approx);
CHECK_COMPLEX_CUSTOM_APPROX(
CHECK_ITERABLE_CUSTOM_APPROX(
goldberg_modes.data()[Spectral::Swsh::goldberg_mode_index(l_max, 3, 3)],
expected_33_mode, bondi_approx);
CHECK_COMPLEX_CUSTOM_APPROX(
CHECK_ITERABLE_CUSTOM_APPROX(
goldberg_modes.data()[Spectral::Swsh::goldberg_mode_index(l_max, 3, -3)],
-expected_33_mode, bondi_approx);
}
Expand Down
Loading
Loading