sxs-collaboration / spectre

SpECTRE is a code for multi-scale, multi-physics problems in astrophysics and gravitational physics.
https://spectre-code.org
Other
161 stars 191 forks source link

Should we have DataBox tags in NumericalAlgorithms/? #2552

Open nilsvu opened 4 years ago

nilsvu commented 4 years ago

I wonder whether we should make it a rule that NumericalAlgorithms/ should be independent of DataBox, and, by extension, should not define DataBox tags. The reasoning is that DataBox is a data structure designed for our parallel algorithms. We could even move DataBox to Parallel/ or ParallelAlgorithms/.

Concerning tags: I found it useful to make the distinction between defining a type (say, just a size_t, to give the most extreme example) and the tags that give this type meaning in a particular context (e.g. an iteration ID in an iterative parallel algorithm). So the type would be defined in DataStructures/ or NumericalAlgorithms/, for example, and the tags would be placed in the context that gives them meaning, which would generally be in ParallelAlgorithms/, Evolution/ or Elliptic/. This means multiple tags can hold the same type, but they may mean different things.

wthrowe commented 4 years ago

Most of SpECTRE is designed to be used in parallel. Why move DataBox in particular?

kidder commented 4 years ago

DataBox should remain in DataStructures as that is what it is, essentially a tagged tuple with some of its elements lazily computed on demand. There is also nothing parallel about a DataBox.

I would suggest that actions and parallel components in NumericalAlgorithms be moved to ParallelAlgorithms and this would allow most tags to be moved as well. I am not convinced that NumericalAlgorithms should be tag free.

nilsdeppe commented 4 years ago

I echo what @wthrowe and @kidder said. DataBox is a data structure that has no parallelization inside of it, hence it is clearly not parallel and should not be in anything named Parallel. I think moving DataBox into ParallelAlgorithm would be a very disastrous move for code organization and would lead to many unneeded dependencies. I vote a very, very strong "definitely not" on this.

I don't understand why you want to forbid having tags in NumericalAlgorithm. There are plenty of things there where associated tags are perfectly fine to use in serial. E.g. why can't I take a divergence on only one core? Why should so much code depend on things in Parallel*? Furthermore, if you need extra tags for a specific action then you can put those in the appropriate Parallel* directory because they aren't used by anything that can be run in serial. Here I also vote "no" because I don't see how it improves anything, and can definitely see how it makes things worse.

Note: by "in serial" I basically mean "without using an action"