Tridiagonal Solver#1234
Conversation
Yurlungur
left a comment
Yurlungur
left a comment
There was a problem hiding this comment.
Looks reasonable. Also looks like it might be incomplete? Is everything in here you want? And should it be tested in the CI?
|
|
||
| auto [rebuild, nbound] = CheckReceiveBufferCacheForRebuild<bound_type, false>(md); | ||
|
|
||
| if (rebuild) { |
There was a problem hiding this comment.
Is this stuff no longer necessary?
There was a problem hiding this comment.
That should be removed from this PR since it isn't relevant to the solver (it got included in the PR by mistake), but I think the change may actually be necessary. There was a bug I ran into a few weeks ago that was fixed by this change, but I never was able to fully wrap my head around what was going on.
The solver as written is complete (it is very simple), but the changes to |
pgrete
left a comment
pgrete
left a comment
There was a problem hiding this comment.
LGTM
other minor comments:
- how about adding a comment somewhere at the top of the file that this is just for testing purpose (and already mention the restrictions in addition to the already implemented hard checks)?
- how hard/useful would it be to add some kind of (simple) test that makes use of the solver (motivated by the comment in the description that this was useful to test some part of the infrastructure downstream)?
| std::string container_base; | ||
| // User defined container in which the solution will reside, only needs to contain | ||
| // sol_fields | ||
| // TODO(LFR): Also allow for an initial guess to come in here |
| // Since this needs to be sequential, we launch an outer loop of size one. Obviously | ||
| // this would be really inefficient on device | ||
| parthenon::par_for( | ||
| DEFAULT_LOOP_PATTERN, "DotProduct", DevExecSpace(), 0, 0, | ||
| KOKKOS_LAMBDA(const int) { |
There was a problem hiding this comment.
Could this be replaced by a parallel_scan (in principle)?
| DEFAULT_LOOP_PATTERN, "PrintSolution", DevExecSpace(), 0, 0, kb.s, kb.e, jb.s, | ||
| jb.e, ib.s - 1, ib.e + 1, | ||
| KOKKOS_LAMBDA(const int b, const int k, const int j, const int i) { | ||
| printf("row %i: %e %e %e %e \n", i, pack_u(b, 0, k, j, i), |
| Real GetSquaredResidualSum() const { return 0.0; } | ||
| int GetCurrentIterations() const { return 1; } |
There was a problem hiding this comment.
This is hardcoded because it's not needed for the direct, single block solve, isn't it?
3 participants
PR Summary
This PR implements a tridiagonal solver within the Parthenon solver framework. This is meant for testing only and will only work for one-dimensional, single block runs. Additionally, it will only work for matrices that extend only one zone away in each direction. The advantage of this solver is that it is direct, so it sometimes allows for disentangling issues related to iterative solver convergence from downstream code implementation issues.
PR Checklist