PCMS interpolation functionality #148
Conversation
Interpolator
searches and adapts radii until all targets get the minimum number of supports and debug queue (counter overflow)
assertion for nan added for interpolated values and formatted
added some checks too that ends up breaking the code and some debugging helper functions
added kokkos kernels LU solver
This is a workaround for multiple definition errors. Functions that are not templated should be moved to the cpp files.
|
@jacobmerson Thanks for the heads up. Thursday or Friday works for me. |
| // It stores the solution in rhs vector itself | ||
|
|
||
| SolveMatrix(result.square_matrix, result.transformed_rhs, team); | ||
|
|
There was a problem hiding this comment.
I noticed some style issues like this previously. I am talking about the blank lines between each line of code.
| using namespace std; | ||
| using namespace Omega_h; | ||
|
|
||
| class queue |
There was a problem hiding this comment.
The class name could be Queue (PascalCase) to follow the PCMS style.
| linear_interpolant.hpp | ||
| multidimarray.hpp | ||
| MLS_rbf_options.hpp | ||
| ) |
There was a problem hiding this comment.
We need to rename these header files to have consistent naming.
| #cmake_minimum_required(VERSION 3.20) | ||
| #project(Interpolator) | ||
|
|
||
| find_package(KokkosKernels REQUIRED) |
There was a problem hiding this comment.
Is there a minimum required version? This should be pinned to the same version as what we use for Kokkos .
| target_link_libraries(pcms_interpolator PUBLIC pcms::core Kokkos::kokkoskernels) | ||
|
|
||
| target_include_directories(pcms_interpolator INTERFACE | ||
| $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}> |
There was a problem hiding this comment.
We should get rid of this. We always want to access header files as pcms/interpolator/...
| #include "points.hpp" | ||
| #include <type_traits> | ||
|
|
||
| #define PI_M 3.14159265358979323846 |
There was a problem hiding this comment.
We should not define our own pi. We can use M_PI from cmath
|
|
||
| #define PI_M 3.14159265358979323846 | ||
|
|
||
| static constexpr int MAX_DIM = 3; |
There was a problem hiding this comment.
Let's reassess if we need this. If we do it should be a compile time setting. It should go into a settings or macro header file that gets configured.
|
|
||
| // Initialize the scratch matrices and vectors | ||
|
|
||
| Kokkos::parallel_for(Kokkos::TeamThreadRange(team, basis_size), |
There was a problem hiding this comment.
Could this be done with kokkos kernels fill instead? Intent would be more clear.
| rbf_func); | ||
| }); | ||
|
|
||
| // // sum phi |
There was a problem hiding this comment.
Remove commented code.
| #include "MLSInterpolation.hpp" | ||
| #include <Omega_h_fail.hpp> | ||
|
|
||
| // TODO add PCMS namespace to all interpolation code |
There was a problem hiding this comment.
This should be done before merge.
| @@ -0,0 +1,23 @@ | |||
| #ifndef MLS_RBF_OPTIONS_HPP | |||
| #define MLS_RBF_OPTIONS_HPP | |||
There was a problem hiding this comment.
Prefix with PCMS_INTERPOLATOR
| using HostIntVecView = Kokkos::View<int*, Kokkos::HostSpace>; | ||
|
|
||
| KOKKOS_INLINE_FUNCTION | ||
| int calculateIndex(const IntVecView& dimensions, const int* indices) |
There was a problem hiding this comment.
Why do we have these. Kokkos views should handle indexing for us.
This pull request brings in @abhiyan123 initial changes for dealing with RBF field interpolations. I have gotten this into a state where it can be merged with develop and tests/compilation passes.
However, I have not done a formal review of this code and there are a some issues that need to be addressed before merging with develop.
@Fuad-HH I have requested you as a reviewer. Please take a look and see if you have any input, questions, etc.
@cwsmith relevant code for our discussion on surface fitting is here. Unfortunately, getting everything merged took some time and I have not had time to do the proper review I wanted before our meeting. Let me know if you prefer to push back that discussion until Thursday/Friday.