refactoring, added comments

wireworld/CMakeLists.txt

@@ -17,7 +17,7 @@ set(NAME ${DWARF_PREFIX}_wireworld)
 if (MPI_FOUND AND Boost_FOUND)
 	enable_language(CXX)
     include_directories(${MPI_INCLUDE_PATH})
-    add_executable(${NAME} main.cpp Configuration.cpp Communicator.cpp FileIO.cpp MpiEnvironment.cpp MpiSubarray.cpp MpiWireworld.cpp Tile.cpp)
+    add_executable(${NAME} main.cpp Configuration.cpp Communicator.cpp FileIO.cpp MpiEnvironment.cpp MpiSubarray.cpp MpiWireworld.cpp Tile.cpp Util.cpp)
     set(CMAKE_BUILD_TYPE RelWithDebInfo)
 	add_definitions(-DGSL_UNENFORCED_ON_CONTRACT_VIOLATION)
     if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")

wireworld/Communicator.cpp

@@ -4,11 +4,18 @@
 
 #include "Communicator.hpp"
 
-void Communicator::ReportInvalidCommunicationModeError() const {
-	std::cerr << "Invalid Communication Mode\n";
-	MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
+Communicator::MpiRequest::MpiRequest(DoubleVector<MPI_Request> reqs)
+    : _reqs(reqs) {}
+
+void Communicator::MpiRequest::Wait() {
+	MPI_Waitall(_reqs.size(), _reqs.data(), MPI_STATUSES_IGNORE);
+	finished = true;
+}
+Communicator::MpiRequest::~MpiRequest() {
+	if (!finished) { MpiReportErrorAbort("Forgot to Wait for MPI_Request"); }
 }
 
+// defines types and graph topology
 Communicator::Communicator(const MpiEnvironment& env,
                            CommunicationMode commMode, const Size& gridSize,
                            const Size& tileSize)
@@ -23,7 +30,8 @@ Communicator::Communicator(const MpiEnvironment& env,
 
 	// End definition of basic types
 
-	// Begin definition of datastructures for a general cell
+	// Begin definition of types/displacements for a general cell somewhere in
+	// the middle of the grid
 	const std::array<MPI_Datatype, NoNeighbors> generalSendTypes{
 	    _haloCornerType, _haloRowType,    _haloCornerType, //
 	    _haloColumnType, _haloColumnType,                  //
@@ -32,11 +40,10 @@ Communicator::Communicator(const MpiEnvironment& env,
 	const auto tCols = tileSize.Cols;
 	const auto tRows = tileSize.Rows;
 
-	const auto dp =
-	    [&](std::size_t x,
-	        std::size_t y) { // character coordinates to displacement
-		    return static_cast<MPI_Aint>(y * (tCols + 2) + x);
-		};
+	// character coordinates to displacement
+	const auto dp = [&](std::size_t x, std::size_t y) {
+		return static_cast<MPI_Aint>(y * (tCols + 2) + x);
+	};
 
 	const std::array<MPI_Aint, NoNeighbors> generalSendDisplacements{
 	    dp(1, 1),     dp(1, 1),     dp(tCols, 1),    //
@@ -57,7 +64,8 @@ Communicator::Communicator(const MpiEnvironment& env,
 	};
 	// End definition of datastructures for a general cell
 
-	// Begin definition of datastructures for this particular cell
+	// Begin definition of datastructures for this particular cell (handle the
+	// border cases)
 	const auto rank2coord = [&](std::size_t rank) {
 		return Coord{
 		    rank % gridSize.Cols, //
@@ -72,7 +80,7 @@ Communicator::Communicator(const MpiEnvironment& env,
 	};
 
 	const auto myCoord = rank2coord(env.worldRank());
-	const std::array<Coord, NoNeighbors> virtualNeighborCoords{{
+	const std::array<Coord, NoNeighbors> generalNeighborCoords{{
 	    {myCoord.X - 1, myCoord.Y - 1}, // intentional signed underflow
 	    {myCoord.X + 0, myCoord.Y - 1}, //
 	    {myCoord.X + 1, myCoord.Y - 1}, //
@@ -84,7 +92,7 @@ Communicator::Communicator(const MpiEnvironment& env,
 	}};
 
 	for (std::size_t i{0}; i < NoNeighbors; ++i) {
-		const auto nbrCoord = virtualNeighborCoords[i];
+		const auto nbrCoord = generalNeighborCoords[i];
 		if (isInsideGrid(nbrCoord)) {
 			_neighbors.push_back(coord2rank(nbrCoord));
 			_sendTypes.push_back(generalSendTypes[i]);
@@ -140,7 +148,7 @@ Communicator& Communicator::operator=(Communicator&& other) noexcept {
 
 void Communicator::Communicate(gsl::multi_span<State, -1, -1>& model) {
 	if (_commDistGraph == MPI_COMM_NULL)
-		throw std::logic_error("Communicator not initialized");
+		MpiReportErrorAbort("Communicator not initialized");
 
 	switch (_commMode) {
 	case CommunicationMode::Collective:
@@ -160,29 +168,15 @@ void Communicator::Communicate(gsl::multi_span<State, -1, -1>& model) {
 		AsyncCommunicate(model).Wait();
 	} break;
 	default:
-		ReportInvalidCommunicationModeError();
+		MpiReportErrorAbort("Invalid Communication mode");
 		break;
 	}
 }
 
-Communicator::MpiRequest::MpiRequest(DoubleVector<MPI_Request> reqs)
-    : _reqs(reqs) {}
-
-void Communicator::MpiRequest::Wait() {
-	MPI_Waitall(_reqs.size(), _reqs.data(), MPI_STATUSES_IGNORE);
-	finished = true;
-}
-Communicator::MpiRequest::~MpiRequest() {
-	if (!finished) {
-		std::cerr << "Forgot to Wait for MPI_Request\n";
-		MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
-	}
-}
-
 Communicator::MpiRequest
 Communicator::AsyncCommunicate(gsl::multi_span<State, -1, -1>& model) {
 	if (_commDistGraph == MPI_COMM_NULL)
-		throw std::logic_error("Communicator not initialized");
+		MpiReportErrorAbort("Communicator not initialized");
 
 	switch (_commMode) {
 	case CommunicationMode::Collective: {
@@ -229,8 +223,7 @@ Communicator::AsyncCommunicate(gsl::multi_span<State, -1, -1>& model) {
 		return MpiRequest{reqs};
 	} break;
 	default:
-		ReportInvalidCommunicationModeError();
+		MpiReportErrorAbort("Invalid Communication mode");
 		break;
 	}
-	return MpiRequest{{}};
 }
\ No newline at end of file

wireworld/Communicator.hpp

@@ -9,13 +9,37 @@
 #include "State.hpp"
 #include "Util.hpp"
 
+// creates the graph topology and does the communication
 class Communicator {
 	constexpr static std::size_t NoNeighbors{8};
 
+	// for very small container with known max size, this is a performance
+	// improvement
 	template <typename T>
 	using Vector = boost::container::static_vector<T, NoNeighbors>;
 
+  public:
+	// Life cycle handling for MPI_Requests
+	class MpiRequest { // TODO: unnest
+	  public:
+		template <typename T>
+		using DoubleVector =
+		    boost::container::static_vector<T, NoNeighbors * 2>;
+
+	  private:
+		DoubleVector<MPI_Request> _reqs;
+		bool finished{};
+
+	  public:
+		MpiRequest(DoubleVector<MPI_Request> reqs);
+		void Wait();
+		~MpiRequest();
+	};
+
+  private:
 	CommunicationMode _commMode;
+
+	// data members for graph topology
 	Vector<int> _neighbors;
 	Vector<int> _sizes;
 	Vector<MPI_Datatype> _sendTypes;
@@ -24,42 +48,22 @@ class Communicator {
 	Vector<MPI_Aint> _recvDisplacements;
 	MPI_Comm _commDistGraph{MPI_COMM_NULL};
 
-	MPI_Datatype _haloRowType;
-	MPI_Datatype _haloColumnType;
+	// data types
+	MPI_Datatype _haloRowType{};
+	MPI_Datatype _haloColumnType{};
 	MPI_Datatype _haloCornerType{MPI_CHAR};
 
-	void ReportInvalidCommunicationModeError() const;
-
   public:
 	Communicator() = default;
 	Communicator(const MpiEnvironment& env, CommunicationMode commMode,
 	             const Size& gridSize, const Size& tileSize);
 	~Communicator();
-
 	void swap(Communicator& first, Communicator& second);
-
 	Communicator(Communicator&) = delete;
 	Communicator& operator=(Communicator&) = delete;
 	Communicator(Communicator&& other) noexcept;
 	Communicator& operator=(Communicator&& other) noexcept;
 
 	void Communicate(gsl::multi_span<State, -1, -1>& model);
-
-	class MpiRequest {
-	  public:
-		template <typename T>
-		using DoubleVector =
-		    boost::container::static_vector<T, NoNeighbors * 2>;
-
-	  private:
-		DoubleVector<MPI_Request> _reqs;
-		bool finished{};
-
-	  public:
-		MpiRequest(DoubleVector<MPI_Request> reqs);
-		void Wait();
-		~MpiRequest();
-	};
-
 	MpiRequest AsyncCommunicate(gsl::multi_span<State, -1, -1>& model);
 };
\ No newline at end of file

wireworld/Configuration.cpp

 #include "Configuration.hpp"
 
+#include <algorithm>
+#include <array>
 #include <iostream>
 #include <stdexcept>
+#include <tuple>
 
 #include "MpiEnvironment.hpp"
 #include <boost/format.hpp>
 #include <boost/program_options.hpp>
 
+// BEGIN helper functions to parse Communication Mode cmd args
 using namespace std::string_literals;
 std::array<std::pair<std::string, CommunicationMode>, 2>
     StringToCommunicationMode{
@@ -26,10 +30,10 @@ std::istream& operator>>(std::istream& in, CommunicationMode& comm) {
 	comm = r->second;
 	return in;
 }
+// END helper functions
 
 auto Configuration::parseArgs(int argc, char* argv[], const MpiEnvironment& env)
     -> Configuration {
-	// parse cmd args
 	namespace po = boost::program_options;
 	Configuration cfg;
 
@@ -51,14 +55,18 @@ auto Configuration::parseArgs(int argc, char* argv[], const MpiEnvironment& env)
 	poDesc.add("inputfile", 1);
 
 	po::variables_map vm;
-	po::store(po::command_line_parser(argc, argv)
-	              .options(desc)      //
-	              .positional(poDesc) //
-	              .run(),
-	          vm);
+	try {
+		po::store(po::command_line_parser(argc, argv)
+		              .options(desc)      //
+		              .positional(poDesc) //
+		              .run(),
+		          vm);
 
-	if (vm.count("help") > 0 && env.isMaster()) { std::cout << desc << "\n"; }
-	po::notify(vm);
+		if (vm.count("help") > 0 && env.isMaster()) {
+			std::cout << desc << "\n";
+		}
+		po::notify(vm);
+	} catch (const po::error& err) { MpiReportErrorAbort(err.what()); }
 
 	// if no dimensions given, use MPI_Dims_create
 	if (cfg.Grid.Cols < 1 || cfg.Grid.Rows < 1) {
@@ -76,10 +84,10 @@ auto Configuration::parseArgs(int argc, char* argv[], const MpiEnvironment& env)
 	const auto& totalCellCount = cfg.Grid.Cols * cfg.Grid.Rows;
 	const auto& worldSize = env.worldSize();
 	if (totalCellCount != static_cast<std::size_t>(worldSize)) {
-		std::cerr << boost::format("Total number of cells (%d) does not match "
-		                           "the MPI World size (%d)") %
-		                 totalCellCount % worldSize;
-		MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
+		MpiReportErrorAbort(boost::str(
+		    boost::format("Total number of cells (%d) does not match "
+		                  "the MPI World size (%d)") %
+		    totalCellCount % worldSize));
 	}
 	return cfg;
 }

wireworld/Configuration.hpp

 #pragma once
 
-#include <algorithm>
-#include <array>
-#include <boost/program_options.hpp>
 #include <cstddef>
-#include <istream>
 #include <string>
-#include <tuple>
 
 #include "MpiEnvironment.hpp"
 #include "Util.hpp"
@@ -17,7 +12,6 @@ enum class CommunicationMode {
 };
 
 struct Configuration {
-
 	Size Grid{};
 	std::string InputFilePath;
 	std::string OutputFilePath;

wireworld/FileIO.cpp

@@ -8,15 +8,15 @@
 
 HeaderInfo FileIO::ReadHeader(const std::string& path) {
 	// read the header into a buf (20 should be sufficient)
+	constexpr auto HeaderBufSize = 20;
 	MPI_File fh;
 	MPI_File_open(MPI_COMM_SELF, path.c_str(),
 	              MPI_MODE_RDONLY | MPI_MODE_UNIQUE_OPEN, MPI_INFO_NULL, &fh);
-	constexpr auto HeaderBufSize = 20;
 	std::array<char, HeaderBufSize> buf;
 	MPI_File_read_all(fh, buf.data(), buf.size(), MPI_CHAR, MPI_STATUS_IGNORE);
 	MPI_File_close(&fh);
 
-	// make stream out of buf
+	// make stream from buf
 	std::istringstream input;
 	input.rdbuf()->pubsetbuf(buf.data(), buf.size());
 
@@ -25,8 +25,7 @@ HeaderInfo FileIO::ReadHeader(const std::string& path) {
 	std::size_t noRows{};
 	input >> noCols >> noRows;
 	if (noCols < 1 || noRows < 1) {
-		std::cerr << "File header corrupt\n";
-		MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
+		MpiReportErrorAbort("File header corrupt");
 	}
 	std::string dummy; // skip line break
 	std::getline(input, dummy);
@@ -34,7 +33,7 @@ HeaderInfo FileIO::ReadHeader(const std::string& path) {
 	return {
 	    {noCols, noRows},
 	    static_cast<std::size_t>(
-	        headerLength) // should fit (is max HeaderBuf size)
+	        headerLength) // should fit (max is HeaderBuf size)
 	};
 }
 
@@ -42,19 +41,19 @@ void FileIO::WriteHeader(const HeaderInfo& header, const std::string& path,
                          const MpiEnvironment& env) {
 	if (!env.isMaster()) return;
 
+	// write header into buffer
 	std::ostringstream ss;
 	ss << header.GlobalSize.Cols << ' ' << header.GlobalSize.Rows << '\n';
 	const auto buf = ss.str();
 
+	// write buffer
 	MPI_File fh;
 	MPI_File_open(MPI_COMM_SELF, path.c_str(),
 	              MPI_MODE_WRONLY | MPI_MODE_CREATE, MPI_INFO_NULL, &fh);
-
 	MPI_File_write(fh, buf.data(), buf.size(), MPI_CHAR, MPI_STATUS_IGNORE);
 	MPI_File_close(&fh);
 }
 
-// TODO: is not an IO concern
 Size FileIO::GetTileSize(Size globalSize, Size gridSize) {
 	const auto tileSizeCols = globalSize.Cols / gridSize.Cols;
 	const auto tileSizeRows = globalSize.Rows / gridSize.Rows;

wireworld/FileIO.hpp

@@ -7,8 +7,8 @@
 
 #include "MpiEnvironment.hpp"
 #include "MpiSubarray.hpp"
-#include "Util.hpp"
 #include "State.hpp"
+#include "Util.hpp"
 
 struct HeaderInfo {
 	Size GlobalSize;
@@ -21,9 +21,9 @@ struct FileIO {
 	static void WriteHeader(const HeaderInfo& header, const std::string& path,
 	                        const MpiEnvironment& env);
 
-	// TODO: is not an IO concern
 	static Size GetTileSize(Size globalSize, Size gridSize);
 
+	// helper class to share commonly used data for reading and writing
 	class Tile {
 		static constexpr std::size_t LF = 1; // linefeed chars
 		const std::string& _path;
@@ -45,7 +45,6 @@ struct FileIO {
 		     std::size_t rank, gsl::span<State> buf);
 
 		void Read();
-
 		void Write() const;
 	};
 };

wireworld/MpiSubarray.cpp

@@ -2,7 +2,7 @@
 
 SubarrayDefinition::SubarrayDefinition(
     std::initializer_list<SubarrayDimensionDefinition> saDimDefs) {
-	for (auto& dd : saDimDefs) {
+	for (const auto& dd : saDimDefs) {
 		_sizes.push_back(static_cast<int>(dd.size));
 		_subSizes.push_back(static_cast<int>(dd.subSize));
 		_starts.push_back(static_cast<int>(dd.start));

wireworld/MpiWireworld.cpp

@@ -60,7 +60,7 @@ MpiWireworld::MpiWireworld(const MpiEnvironment& env, const Configuration& cfg)
 }
 
 std::ostream& operator<<(std::ostream& out, const MpiWireworld& g) {
-	// for now, print the own tile
+	// for now, put only our local tile
 	out << g._tile;
 	return out;
 }

wireworld/MpiWireworld.hpp

@@ -17,10 +17,7 @@ class MpiWireworld {
 
   public:
 	MpiWireworld(const MpiEnvironment& env, const Configuration& cfg);
-
 	friend std::ostream& operator<<(std::ostream& out, const MpiWireworld& g);
-
 	void write() const;
-
 	void simulateStep();
 };
\ No newline at end of file

wireworld/Tile.cpp

@@ -13,8 +13,8 @@ Tile::Tile(const Configuration& cfg, const MpiEnvironment& env)
       _tileSize(FileIO::GetTileSize(_header.GlobalSize, cfg.Grid)) {
 	if ((_header.GlobalSize.Cols % _tileSize.Cols) != 0 ||
 	    (_header.GlobalSize.Rows % _tileSize.Rows) != 0) {
-		std::cerr << "Wireworld size is not even divisible by the grid size.\n";
-		MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
+		MpiReportErrorAbort(
+		    "Wireworld size is not even divisible by the grid size.");
 	}
 	const auto bufsize = (_tileSize.Cols + 2) * (_tileSize.Rows + 2);
 	_memoryA.resize(bufsize);

wireworld/Util.cpp

+#include "Util.hpp"
+
+#include <mpi.h>
+
+#include <iostream>
+
+void MpiReportErrorAbort(const std::string& err) {
+	std::cerr << err << '\n';
+	MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE);
+	std::abort(); // workaround for compiler warnings
+}
\ No newline at end of file

wireworld/Util.hpp

 #pragma once
 
 #include <cstddef>
-
+#include <string>
 struct Size {
 	std::size_t Cols{};
 	std::size_t Rows{};
@@ -10,4 +10,6 @@ struct Size {
 struct Coord {
 	std::size_t X{};
 	std::size_t Y{};
-};
\ No newline at end of file
+};
+
+[[noreturn]] void MpiReportErrorAbort(const std::string& err);
\ No newline at end of file

wireworld/main.cpp

@@ -22,7 +22,6 @@ int main(int argc, char* argv[]) {
 
 	MpiWireworld ww(env, cfg);
 
-
 	for (std::size_t i{0}; i < cfg.Generations; ++i) {
 		ww.simulateStep();