Skip to content

Conversation

dkachuma
Copy link
Contributor

@dkachuma dkachuma commented Dec 19, 2023

The data layout in constitutive::multifluid depends on the number of dimensions in the array. This PR removes the explicit references to the layout by creating shorthand data types in one place.

@dkachuma dkachuma self-assigned this Dec 19, 2023
@dkachuma dkachuma added the type: cleanup / refactor Non-functional change (NFC) label Dec 19, 2023
@klevzoff
Copy link
Contributor

klevzoff commented Dec 19, 2023

@dkachuma for historical perspective: we intentionally wanted to be able to specify memory layouts for different types of fluid properties independently of each other, in case it turned out to be important for optimizing kernels (which might have different access patterns for different data). The only constant is that cell dimension must be contiguous so long as we are threading on cell index; but the effect of e.g. KJI vs KIJ layout was never investigated. In the end nobody spent any time on it because we are bottlenecked by linear solver anyway... until somebody comes up with a compositional matrix-free solver :)

In the long term, I was also planning to introduce some changes to our layout specifications that would allow us to do things like embedding compile-time constants in the layout (say, after dispatching on number of components and knowing statically NC=5, we would make a bunch of views with a static-5 dimension). This would likely cause all layouts in kernel code to be deduced rather that hardcoded.

None of this means I oppose the changes in this PR. Ergonomics for developers is also an important consideration :)

@dkachuma dkachuma closed this Apr 15, 2024
@dkachuma dkachuma deleted the refactor/dkachuma/multiphase-fluid-base branch April 15, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: cleanup / refactor Non-functional change (NFC)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants