Move fields that are only sometimes needed to the cache?

[efaulhaber]

Okay, how about this?
We reduce fields to the minimum, and everything that is only needed when some method is used goes to the cache. Then we define

@inline function color(system)
    @assert haskey(system.cache, :color) "`color` of a `$(nameof(typeof(system)))` accessed, but there is no color method defined for this system"
    return system.cache.color
end

We could do the same for example with buffer, which is only needed by open boundaries and set to nothing in most simulations. I think this could make the system structs more readable.
What do you think, @LasNikas?

Originally posted by @efaulhaber in #539 (comment)

[LasNikas]

Yes, that's a good idea.
Also, the constructor of the systems is starting to grow with argument error checking and creating caches etc.
Should we discuss a solution for this as well? I was thinking of something like in Semidiscretization with the check_configuration. It's clear and doesn't mess the constructor.

[efaulhaber]

Yes, check_configuration is a good idea for the systems as well.
I suggested earlier that we add a kwarg techniques or so to be used as follows.

WeaklyCompressibleSPHSystem(..., techniques=(DensityDiffusionMolteniColagrossi(delta=0.1),
                                             KernelGradientCorrection(),
                                             TransportVelocityFormulation())

Most of our current kwargs are optional techniques to improve simulations.

[svchb]

One downside is that it is not clear which system supports which fields. So this seems to me more a thing that should be done maybe in a 0.3 refactor?

[efaulhaber]

Well, it should come with a nice table in the docs listing all available techniques vs all available systems, and then showing ✅ or ❌ for each combination, similar to the README of PointNeighbors..

[sloede]

You can even make this type stable if you want, by adding type-based traits. Similar to what we have done with have_nonconservative_terms in Trixi.jl.

That's an optimization though and should only be used when type stability is (can be) an issue.

[efaulhaber]

Why wouldn't the Tuple be type stable?

[sloede]

I'm pretty sure that the @assert does some Weird Stuff under the hood. At least the code path for a system not containing color I suspect of being not type stable. But it also depends on how you use it - if it is strictly for catching errors then it might be ok

[efaulhaber]

Ah you were referring to the code in the first comment. I checked the LLVM when passing a tuple, and the check is indeed optimized away.