felsenhower / partdiffpp Goto Github PK

Compare pde's and pde++' performance

Remove strictly procedural style functions and make them member functions of the respective structs / classes

Use bool instead of int

In descending priority

• OpenMP
• MPI
  • Charm++' Adaptive MPI (AMPI)
• OpenACC

At the moment, allocateMatrices allocates one big memory block double *M with dimensions num_matrices * (N + 1)² and inits double ***Matrix such that the elements of Matrix point to the elements of M, i.e. Matrix[0][0][0] == M[0], Matrix[0][0][1] == M[1] and so on.

This is good, because it guarantees that adjacent elements inside Matrix are adjacent in M and therefore adjacent in memory. This is cache-friendly, because the calculation of star inside calculate uses four close elements.

However, the pointer dereferencing brings a little overhead and one could replace it with doing the pointer arithmetic directly.

This could be done with a class Matrix that accepts two indices like

Matrix[x,y] = value; // or....
Matrix(x,y) = value;

and a vector<Matrix> or a class Tensor that directly accepts three (or better an arbitrary number) of indices.

It is not possible at the current time to directly overload the operator[] to accept two indices: https://stackoverflow.com/questions/1936399/c-array-operator-with-multiple-arguments

There are ways to still do this nonetheless (https://stackoverflow.com/questions/5602112/when-to-overload-the-comma-operator/18136340#18136340), but they involve maps of pairs which probably have a worse performance than arrays.

Use static constexpr or enum class instead

Reduce the scope of all variables as much as possible.

Initialize every variable directly at declaration.

The original partdiff had rich comments and headers, I'd like to reintroduce them at some point in an amended way.

Replace NULL with nullptr

Remove void pointer arithmetic and replace with

• char pointer arithmetic where necessary
• std::any

Currently, all IO is done via streams and all temporary streams are used via the build_string method:

std::string build_string(const std::function<void(std::stringstream &)> input) {
  std::stringstream ss;
  input(ss);
  return ss.str();
}

std::cout << [...]
              << partdiff::build_string([time](auto &ss) { ss << std::fixed << std::setprecision(6) << time; }) << " s"
              << std::endl

While this is a very C++ way of exploiting streams, it's also verbose compared to how printing works in e.g. Python.

C++20 adds a format library that adds Python-style printing which sadly isn't yet implemented in g++ 10.2.0.

However, there is the fmt library that implements it.

We could use that and switch to std::format once that's available.

partdiff uses double everywhere.

Investigate the accuracy / performance tradeof of just using float.

If float can be used everywhere, maybe define a type "real" that maps to double or float.

This is not a stylistic choice, but a practical one:

In C, you declare variables of a struct like this:

struct foo { ... }
struct foo bar;

In C++, the word struct can be omitted in the variable declaration:

struct foo { ... }
foo bar;

If the type and variable name are the same (like frequently in partdiff, e.g. options), this can lead to ambiguities.

The only solution to have to problems at all are to keep the two seperate, e.g. like this:

struct calculation_arguments_t { ... }
calculation_arguments_t calculation_arguments;

Or like this:

struct CalculationArguments { ... }
CalculationArguments calculation_arguments

partdiff uses the word accuracy quite often.

Unlike the German "Präzision", this is not the correct word. Instead, accuracy should be used when not compiled in legacy mode.

https://en.wikipedia.org/wiki/Accuracy_and_precision

Replace exit(0) with exit(EXIT_SUCCESS)

Replace exit(1) with exit(EXIT_FAILURE)

One global namespace, and one askparams namespace

The function partdiff::askparams::askParams contains a lot of code duplication

The original partdiff is partly in German and partly in English.

More precisely, everything is in English except for the functions displayStatistics and allocateMemory.

It would be nice to have everything in the application in English, but that would break make test, since that compares the output of the two applications on a char-by-char level.

A solution could be adding a compiler flag, e.g. -Dlegacy or so, that retains all of the original behaviour and translating everything to English if the option has not been passed.

This would also enable us to modernise a few other things here and there.

We use stol when converting an std::string to a uint64_t, but since we can't be sure that uint64_t maps to long, this is a bad practice.

The following defines should be converted to enum class for better type safety:

#define METH_GAUSS_SEIDEL 1
#define METH_JACOBI 2
#define FUNC_F0 1
#define FUNC_FPISIN 2
#define TERM_PREC 1
#define TERM_ITER 2

Remove all imports of C headers if possible

Remove instances of malloc and free and replace them by

• new / delete
• copies of classes
• smart pointers

Switch from {fmt} library to std::format once it becomes available in compilers.

Remove printf and replace with std::cout