Skip to content

Commit

Permalink
clear outstanding warnings (#134)
Browse files Browse the repository at this point in the history
  • Loading branch information
jwaldrop107 authored May 14, 2024
1 parent dbefd4f commit c33ccb7
Show file tree
Hide file tree
Showing 15 changed files with 40 additions and 43 deletions.
2 changes: 1 addition & 1 deletion include/parallelzone/hardware/ram/ram.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace parallelzone::hardware {

namespace detail_ {

class RAMPIMPL;
struct RAMPIMPL;
}

/** @brief Provides a runtime API for interacting with memory
Expand Down
7 changes: 2 additions & 5 deletions include/parallelzone/mpi_helpers/commpp/commpp.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ typename CommPP::gather_return_type<T> CommPP::gather_t_(
auto serialized_size = buffer.size() / size();

value_type vec(size());
for(size_type i = 0, j = 0; i < buffer.size();
for(size_type i = 0, j = 0; i < size_type(buffer.size());
i += serialized_size, ++j) {
const_binary_reference view(binary_rv->data() + i, serialized_size);
vec[j] = std::move(from_binary_view<clean_type>(view));
Expand Down Expand Up @@ -122,8 +122,6 @@ typename CommPP::gather_return_type<T> CommPP::gatherv_t_(
using return_type = typename CommPP::gather_return_type<clean_type>;
using value_type = typename return_type::value_type;

const bool am_i_root = root.has_value() ? me() == *root : true;

if constexpr(needs_serialized_v<clean_type>) {
// Do gather in binary
auto binary = make_binary_buffer(std::forward<T>(input));
Expand Down Expand Up @@ -165,8 +163,7 @@ typename CommPP::gather_return_type<T> CommPP::gatherv_t_(
constexpr auto t_size = sizeof(element_type);

value_type vec;
auto n_elements = buffer.size() / t_size;
for(size_type i = 0; i < buffer.size(); i += t_size) {
for(size_type i = 0; i < size_type(buffer.size()); i += t_size) {
const_binary_reference view(buffer.data() + i, t_size);
vec.emplace_back(std::move(from_binary_view<element_type>(view)));
}
Expand Down
2 changes: 1 addition & 1 deletion include/parallelzone/runtime/resource_set.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
namespace parallelzone::runtime {

namespace detail_ {
class ResourceSetPIMPL;
struct ResourceSetPIMPL;
}

/** @brief A set of runtime resources.
Expand Down
2 changes: 1 addition & 1 deletion include/parallelzone/runtime/runtime_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

namespace parallelzone::runtime {
namespace detail_ {
class RuntimeViewPIMPL;
struct RuntimeViewPIMPL;
}

/** @brief A view of the execution environment.
Expand Down
8 changes: 5 additions & 3 deletions src/parallelzone/runtime/resource_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ ResourceSet::size_type ResourceSet::mpi_rank() const noexcept {

bool ResourceSet::is_mine() const noexcept {
if(!has_pimpl_()) return false;
if(mpi_rank() == MPI_PROC_NULL) return false;
return m_pimpl_->m_my_mpi.me() == mpi_rank();
if(mpi_rank() == size_type(MPI_PROC_NULL)) return false;
return size_type(m_pimpl_->m_my_mpi.me()) == mpi_rank();
}

bool ResourceSet::has_ram() const noexcept { return has_pimpl_() && !null(); }
Expand All @@ -71,7 +71,9 @@ ResourceSet::logger_reference ResourceSet::logger() const {
// -- Utility methods
// -----------------------------------------------------------------------------

bool ResourceSet::null() const noexcept { return mpi_rank() == MPI_PROC_NULL; }
bool ResourceSet::null() const noexcept {
return mpi_rank() == size_type(MPI_PROC_NULL);
}

bool ResourceSet::empty() const noexcept {
// TODO: Need to check for loggers
Expand Down
1 change: 1 addition & 0 deletions tests/cxx/doc_snippets/quickstart.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ TEST_CASE("quick start") {
// Figure out how much RAM the local process has access to
const auto my_ram = my_rs.ram().total_space();

REQUIRE(my_ram >= 0);
REQUIRE(n_rs > 0);
if(my_rs_rank == 0) {
REQUIRE(ranks_on_0);
Expand Down
2 changes: 2 additions & 0 deletions tests/cxx/doc_snippets/ram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,6 @@ TEST_CASE("ram") {
} else {
REQUIRE_FALSE(all_data);
}
REQUIRE(rank_0_total >= 0);
REQUIRE(my_total_ram >= 0);
}
2 changes: 2 additions & 0 deletions tests/cxx/doc_snippets/resource_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,6 @@ TEST_CASE("resource_set") {
REQUIRE(is_local == am_i_supposed_to_be_0);
REQUIRE(do_i_have_ram);
REQUIRE(does_0_have_ram);
REQUIRE(my_ram.total_space() >= 0);
REQUIRE(ram_0.total_space() >= 0);
}
1 change: 0 additions & 1 deletion tests/cxx/unit_tests/parallelzone/hardware/ram/ram.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ TEST_CASE("RAM") {
using size_type = ram_type::size_type;
const auto& run = testing::PZEnvironment::comm_world();
const auto& rs = run.my_resource_set();
auto my_rank = rs.mpi_rank();

RAM defaulted;
RAM has_value = rs.ram();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ TEST_CASE("SpdlogPIMPL") {
severity::info, severity::warn,
severity::error, severity::critical};

for(auto i = 0; i < all_levels.size(); ++i) {
for(decltype(all_levels.size()) i = 0; i < all_levels.size(); ++i) {
SECTION("level = " + std::to_string(i)) {
ss_log.set_severity(all_levels[i]);
ss_log.log(severity::trace, "Hello trace");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ namespace {
template<typename T>
auto byte_data(T&& input) {
using const_pointer = BinaryView::const_pointer;
using size_type = BinaryView::size_type;
using clean_type = std::decay_t<T>;
auto p = reinterpret_cast<const_pointer>(input.data());
auto n = input.size() * sizeof(typename clean_type::value_type);
Expand Down
30 changes: 13 additions & 17 deletions tests/cxx/unit_tests/parallelzone/mpi_helpers/commpp/commpp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@ using size_type = std::size_t;
using opt_root_type = std::optional<size_type>;

TEST_CASE("CommPP") {
using value_type = CommPP::binary_type;
using const_reference = CommPP::const_binary_reference;

auto& world = testing::PZEnvironment::comm_world();

CommPP defaulted;
Expand All @@ -35,8 +32,8 @@ TEST_CASE("CommPP") {
MPI_Comm_size(comm.comm(), &corr_size);
MPI_Comm_rank(comm.comm(), &corr_rank);

auto n_ranks = comm.size();
auto me = comm.me();
auto n_ranks = size_type(comm.size());
auto me = size_type(comm.me());

SECTION("CTors") {
SECTION("Default") {
Expand Down Expand Up @@ -125,13 +122,13 @@ TEST_CASE("CommPP") {
SECTION("size()") {
REQUIRE(defaulted.size() == 0);
REQUIRE(null.size() == 0);
REQUIRE(n_ranks == corr_size);
REQUIRE(n_ranks == size_type(corr_size));
}

SECTION("me()") {
REQUIRE(defaulted.me() == MPI_PROC_NULL);
REQUIRE(null.me() == MPI_PROC_NULL);
REQUIRE(me == corr_rank);
REQUIRE(me == size_type(corr_rank));
}

SECTION("swap") {
Expand Down Expand Up @@ -168,16 +165,15 @@ TEST_CASE("CommPP") {
// These loops test various MPI operations under different roots and
// different message sizes.

// All of these types must be constructible given a single std::size_t
const int max_ranks = 5;
const std::size_t max_chunk_size = 5;
using needs_serialized = std::string;
using no_serialization = double;
// All of these types must be constructible given a single size_type
const size_type max_ranks = 5;
const size_type max_chunk_size = 5;
using needs_serialized = std::string;
using no_serialization = double;

for(std::size_t chunk_size = 1; chunk_size < max_chunk_size; ++chunk_size) {
for(size_type chunk_size = 1; chunk_size < max_chunk_size; ++chunk_size) {
auto chunk_str = " chunk = " + std::to_string(chunk_size);
auto begin = me * chunk_size;
auto end = begin + chunk_size;

SECTION("all gather" + chunk_str) {
SECTION("needs serialized") {
Expand Down Expand Up @@ -205,7 +201,7 @@ TEST_CASE("CommPP") {
data_type local_data(chunk_size * me, "Hello");
auto rv = comm.gatherv(local_data);
std::vector<data_type> corr;
for(std::size_t i = 0; i < n_ranks; ++i) {
for(size_type i = 0; i < n_ranks; ++i) {
corr.emplace_back(data_type(chunk_size * i, "Hello"));
}
REQUIRE(rv == corr);
Expand Down Expand Up @@ -240,7 +236,7 @@ TEST_CASE("CommPP") {
REQUIRE(rv == corr);
}

for(std::size_t root = 0; root < std::min(n_ranks, max_ranks); ++root) {
for(size_type root = 0; root < std::min(n_ranks, max_ranks); ++root) {
auto root_str = " root = " + std::to_string(root);

SECTION("gather " + root_str + chunk_str) {
Expand Down Expand Up @@ -281,7 +277,7 @@ TEST_CASE("CommPP") {

if(me == root) {
std::vector<data_type> corr;
for(std::size_t i = 0; i < n_ranks; ++i) {
for(size_type i = 0; i < n_ranks; ++i) {
corr.emplace_back(
data_type(chunk_size * i, "Hello"));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ auto make_data(std::size_t min, std::size_t max) {
*/
template<typename T>
void gather_kernel(std::size_t chunk_size, root_type root, pimpl_type& comm) {
using reference = pimpl_type::binary_reference;
using const_reference = pimpl_type::const_binary_reference;
using size_type = std::size_t;

auto am_i_root = root.has_value() ? comm.me() == *root : true;
auto data = make_data<T>(0, chunk_size * comm.size());
Expand Down Expand Up @@ -83,7 +81,6 @@ void gather_buffer_kernel(std::size_t chunk_size, root_type root,
pimpl_type& comm) {
using reference = pimpl_type::binary_reference;
using const_reference = pimpl_type::const_binary_reference;
using size_type = std::size_t;

bool am_i_root = root.has_value() ? comm.me() == *root : true;

Expand Down Expand Up @@ -128,7 +125,7 @@ void gatherv_kernel(std::size_t chunk_size, root_type root, pimpl_type& comm) {
REQUIRE(rv.has_value());
std::vector<T> corr;
std::vector<int> sizes_corr; // Sizes (in bytes)
for(std::size_t rank = 0; rank < n_ranks; ++rank) {
for(std::size_t rank = 0; rank < std::size_t(n_ranks); ++rank) {
sizes_corr.push_back((chunk_size + rank) * sizeof(T));
for(std::size_t i = 0; i < chunk_size + rank; ++i)
corr.push_back(T(i + 1));
Expand Down Expand Up @@ -202,7 +199,8 @@ TEST_CASE("CommPPPIMPL") {
gatherv_kernel<double>(chunk_size, std::nullopt, comm);
}

for(std::size_t root = 0; root < std::min(n_ranks, 5); ++root) {
std::size_t min = std::min(n_ranks, 5);
for(std::size_t root = 0; root < min; ++root) {
auto root_str = " root = " + std::to_string(root);
SECTION("gather" + root_str + chunk_str) {
gather_kernel<std::byte>(chunk_size, root, comm);
Expand Down
13 changes: 7 additions & 6 deletions tests/cxx/unit_tests/parallelzone/runtime/resource_set.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ using namespace parallelzone::runtime;
*
*/
TEST_CASE("ResourceSet") {
using pimpl_type = ResourceSet::pimpl_type;
using comm_type = parallelzone::mpi_helpers::CommPP;
using logger_type = ResourceSet::logger_type;
const auto null_rank = MPI_PROC_NULL;
using pimpl_type = ResourceSet::pimpl_type;
using comm_type = parallelzone::mpi_helpers::CommPP;
using logger_type = ResourceSet::logger_type;
using size_type = ResourceSet::size_type;
const size_type null_rank(MPI_PROC_NULL);

logger_type log;

Expand Down Expand Up @@ -59,7 +60,7 @@ TEST_CASE("ResourceSet") {
REQUIRE_FALSE(null.is_mine());
REQUIRE_FALSE(null.has_ram());

REQUIRE(rs.mpi_rank() == comm.me());
REQUIRE(rs.mpi_rank() == size_type(comm.me()));
REQUIRE(rs.is_mine());
REQUIRE(rs.has_ram()); // I presume all computers have RAM...
}
Expand Down Expand Up @@ -112,7 +113,7 @@ TEST_CASE("ResourceSet") {
SECTION("mpi_rank") {
REQUIRE(defaulted.mpi_rank() == null_rank);
REQUIRE(null.mpi_rank() == null_rank);
REQUIRE(rs.mpi_rank() == comm.me());
REQUIRE(rs.mpi_rank() == size_type(comm.me()));
}

SECTION("is_mine") {
Expand Down
2 changes: 1 addition & 1 deletion tests/cxx/unit_tests/parallelzone/runtime/runtime_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ TEST_CASE("RuntimeView") {
REQUIRE_THROWS_AS(null.at(0), std::out_of_range);
auto n_resource_sets = defaulted.size();
logger_type log;
for(auto i = 0; i < n_resource_sets; ++i) {
for(decltype(n_resource_sets) i = 0; i < n_resource_sets; ++i) {
auto corr = runtime::detail_::make_resource_set(i, comm, log);
REQUIRE(defaulted.at(i) == corr);
REQUIRE(argc_argv.at(i) == corr);
Expand Down

0 comments on commit c33ccb7

Please sign in to comment.