API not robust against misuse

[FabienPean-Virtonomy]

Summarizing earlier discussion:

The new approach to use algorithms, e.g. InteractionDynamics<fluid_dynamics::TransportVelocityCorrectionComplex> pushes implementation details knowledge requirement on users and does not prevent misuse. For example, someone using InteractionDynamics instead of InteractionWithUpdate compiles and would generate wrong output without error/warning.

Moreover, there are still several inconsistencies in the codebase, for example solid_dynamics::AverageVelocityAndAcceleration does not require such wrapping.

The problem is that it imposes a weak interface coupling on the inner class with no way to enforce validity. How to use the algorithm is known by the developer and should be enforced directly, allowing only relevant policies to the surface such as particle range (and perhaps parallel scheduling backend in the future).

As I see there are currently 2 options, one simple enough and directly applicable: with alias template (either in a new namespace SPH::, SPH::algorithms::, or move existing into another namespace like impl). The other option would involve larger refactoring with static polymorphism and class policies with no clear (short-term) benefits and harder to formulate so I will skip.

For example, this would yield

namespace fluid_dynamics
{
template<typename Range = SPHBody>
using TransportVelocityCorrectionComplex = InteractionDynamics<impl::TransportVelocityCorrectionComplex,Range>;
}

[Xiangyu-Hu]

Yes. There is a issue like that. The idea form myside for revision is static assert the matching between the algorithm of member functions using type traits.

[FabienPean-Virtonomy]

Alternatively, this can be done with a concept in C++20. But implementing negative constraints is hardly feasible.

Is the algorithm meant to be interchangeable? If whether InteractionDynamics or InteractionWithUpdate cannot/shouldn't be changed for any class anywhere and is meant to be decided by the class developer prior implementation, then this would be preferable to be done via some form of static polymorphism. This ensures that the developer implements the right interface, while not leaking gory implementation details outside.

[Xiangyu-Hu]

I have made a constructor runtime checking using type traits so that local dynamics and algorithm must be matched in member function. The code is now in code refactory branch. I will initiate a pull request soon.

[FabienPean-Virtonomy]

I simplified the detection idiom with its simpler version here https://github.com/Xiangyu-Hu/SPHinXsys/pull/157 Now though, it must be added in all of them and would need to be modified for every new algorithm added. Hopefully they won't change soon.

[Xiangyu-Hu]

Thanks. I can implement the static assert so that a runtime check is not necessary.

[Xiangyu-Hu]

Now, the revised version uses static assert to check the match. We are lucky that we only have only have 4 algorithms need such type traits check. Your version seems need some revision to work (I have tried but did not give error message when algorithm misused).

[FabienPean-Virtonomy]

Yes I messed up the usage by putting the wrong arguments, but you got the gist of it in this commit 913bb24. Your version does not differentiate between member variable and member function, but that shouldn't be an issue. As long as there are less than a handful of algorithms it will do.