Remove bad software design: Singleton

[uphoffc]

SeisSol makes heavy use of the singleton pattern that I consider bad software design. (In particular the "SeisSol" class.) I believe the singletons where initially introduced due to mess in passing state from C++ and Fortran. Since the Fortran part was finally deleted thanks to @davschneller, I think one should also get rid of the singletons.

Rationale

Singletons have the following disadvantages:

• Essentially equivalent to global variables; the big issue with global variables is that it's unclear what the state of the global variable is (initialized? already destroyed / finalized?)
• Strong coupling between components. Many classes don't work without the "SeisSol" singleton, and the SeisSol class couples a large chunk of the code-base
• By design, there can be only one instance of the singleton. Difficult to refactor if one would require more than one instance.
• Difficult to test classes as one often needs to mock the singleton (that would be useful should there be more tests :-D)

Description

I grepped 172 references to "SeisSol::main". Most of the them are found in Initialization, therefore should be not too hard to remove them.

Discussion

I wonder whether you generally agree that singletons are bad software design and should be removed? If yes, we could start cleaning those up.

Which other singletons are currently present in the code-base?

[ravil-mobile]

I agree regarding the SeisSol class.

There are a few more singletons: MPI and Device classes.

SeisSol/src/Parallel/MPI.h

Lines 213 to 214 in c2062e4

	/** The only instance of the class */
	static MPI mpi;

But they are mostly wrappers for C-APIs.

[sebwolf-de]

Hi,
I agree that the singletons are bad design and thanks for taking the initiative here.
In particular, for UQ studies, it would be advantageous to use SeisSol with MPI subcommunicators. SeisSol::MPI is also a singleton, which should be replaced:

SeisSol/src/Parallel/MPI.cpp

Line 127 in c2062e4

seissol::MPI seissol::MPI::mpi;

IMHO: a good goal would be a SeisSol library, which you can use such as

#import <seissol.h>

MPI_comm comm = ...;
SeisSolParams params = ...;
SeisSol simulator(comm);
simulator.run(params);

Maybe, we could combine that with python bindings, such as we already have it for the proxy: https://github.com/SeisSol/SeisSol/tree/master/auto_tuning/proxy/src/proxy-runners.
Although we developers might dislike it, our users love SeisSol in a jupyter notebook ;-)

[davschneller]

Yup, that sounds great.

If we want to go for a full Python interface, we could also think about compiling all materials etc. into one binary in the long run. That would also bring us really close to multi-material simulations. (I tried to get started on that... Cf. this very WIP branch which tries to go there... https://github.com/SeisSol/SeisSol/tree/davschneller/reduce-defines Right now, it doesn't compile however, since e.g. LTS was templated and not yet propagated)

Currently the initialization procedure iirc only depends on the SeisSol main object, i.e. if we just pass it as object, we'd be done with it... But the code would still not look that beautiful—because to be honest, the whole initialization procedure as it is now should be refactored longer-term, since the current translated code is mostly a copy of the FORTRAN initialization (minus the interop)—thus we still have some rather obscure code structure. E.g.: the SeisSol class contains some classes like LTSLayout or MemoryManager which are IMO also used rather inconveniently (most likely due to the same reasons). Especially the translated model initialization is still pretty messy (cf. InitModel), since it contained a lot of FORTRAN->C++->FORTRAN code. (I'm currently already thinking up some replacement for LTSLayout which, if everything goes by plan, should be done soon-ish (but I'll be away for this and the next week) ... At least some of the MPI calls in that class are redundent by now)