new interpolation framework #699
Conversation
mkstoyanov
force-pushed
the
interpolation_scheme
branch
from
01e2d52 to
abb43ce
Compare
|
|
||
| namespace asgard | ||
| { | ||
| #ifdef KRON_MODE_GLOBAL_BLOCK |
There was a problem hiding this comment.
Did you consider conditionally including this file and the associated asgard_interpolation.cpp and asgard_interpolation_tests.cpp based on the CMake flag?
There was a problem hiding this comment.
Doing several answers in one:
This PR has the code that can go through the steps of the interpolation procedure but lacks the link to the PDE and the rest of the code. I want to keep the PRs more manageable instead of pushing that is many thousands of lines (this is >2K already).
In the final design, those files will not be conditional but will be included with every combination of options. To get this right, I will need to reconcile interpolation with local kronmult and that's a mid size rework in it's own. The if-defs will go away, just like any CMake code that I use as an alternative.
The second point is that the connectivity and workspace will be shared with the kronmult matrix, so the owner will be the kron_operatos class that will then hold the interpolation and matrix (for the linear separable case). Right now the kron_operatos is holding the workspace and the matrix takes a non-owning alias. The matrix owns the connectivity but I will pull that one out too.
Connectivity, workspace and the interpolation matrices can be reused even when the refinement and grid change Connectivity and matrices have to change if we add an extra level, otherwise they can stay the same and save on rebuilding/reallocating those. That would be the job of kron_operatos class, which is just more of exactly what it does now.
|
|
||
| namespace asgard | ||
| { | ||
| #ifdef KRON_MODE_GLOBAL_BLOCK |
There was a problem hiding this comment.
same question as with asgard_interpolation, is this better done within CMake?
| { | ||
| if constexpr (n == -1) | ||
| #pragma omp atomic | ||
| number_of_blocks_ += 1; |
There was a problem hiding this comment.
Can one use number_of_blocks_ ++ with omp atomic?
| wavelet_interp1d() : conn(nullptr) | ||
| {} | ||
| //! cache interpolation points and transformation matrices | ||
| wavelet_interp1d(connect_1d const *conn_in) : conn(conn_in) |
There was a problem hiding this comment.
Should wavelet_interp1d own the conn object?
| interpolation(int num_dimenisons, connect_1d const *conn_in, | ||
| kronmult::block_global_workspace<precision> *workspace) |
There was a problem hiding this comment.
ownership of workspace?
Co-authored-by: Steven Hahn <[email protected]>
First draft the interpolation framework, the PR is already large, it's a good idea to merge it before we start piling in features.