Split Smoother Matrix Construction into Separate COO and Tridiagonal Sections#206
Split Smoother Matrix Construction into Separate COO and Tridiagonal Sections#206julianlitz wants to merge 13 commits intomainfrom
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #206 +/- ##
==========================================
- Coverage 90.70% 90.65% -0.06%
==========================================
Files 86 85 -1
Lines 9458 9320 -138
==========================================
- Hits 8579 8449 -130
+ Misses 879 871 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
EmilyBourne
left a comment
There was a problem hiding this comment.
Leaving the first part of my review as I see the same patterns repeating so it will be easier to finish the review when I have the reply to the first set of questions
|
@EmilyBourne We could merge #230 into this branch before merging it into main. |
|
@julianlitz Can you fix the conflicts for this branch? |
If #230 modifies code that is not touched in this branch then I would prefer to merge after this PR instead of merging into this PR to simplify the review. This PR is already enormous (+1,293 -1,533) and therefore hard to review. Let's not make it worse 😅 |
|
@EmilyBourne Resolving conflicts is now done |
Thanks |
|
#230 Eliminates the changes done to DirectSolverCSR in this Pull Request by deleting the class. But apart from that they can be viewed separately. |
|
This PR has mainly that many changes because I needed to move code sections into a different file. But I think it is quite important to merge it. 🙃 |
|
And in this PR I do the necessary adjustments for We did a similar PR a month ago where I did the exact same thing for |
| template <class LevelCacheType> | ||
| void ExtrapolatedSmootherGive<LevelCacheType>::nodeBuildInteriorBoundarySolverMatrix_i_r_0( | ||
| int i_theta, const PolarGrid& grid, bool DirBC_Interior, InnerBoundaryMatrix& matrix, double arr, double att, | ||
| double art, double detDF, double coeff_beta) | ||
| { |
There was a problem hiding this comment.
I suspect that this method was moved from somewhere else, but it has also been renamed so I can't find the original. Can you give me a hint
There was a problem hiding this comment.
Found it. It was extracted from void ExtrapolatedSmootherGive<LevelCacheType>::nodeBuildAscGive in include/ExtrapolatedSmoother/ExtrapolatedSmootherGive/buildAscMatrices.inl
The trouble is that it doesn't show up that way easily so I have to split screen it and find the matching functions to check if it's just a move, or if something changed too. |
|
Basically any matrix entries related to COO/CSR moved to a different file and the i_r=0 treatment for tridiagonal matrices is that they get a 1 diagonal. |
Close #233
Merge Request - GuideLine Checklist
Guideline to check code before resolve WIP and approval, respectively.
As many checkboxes as possible should be ticked.
Checks by code author:
Always to be checked:
If functions were changed or functionality was added:
If new functionality was added:
If new third party software is used:
If new mathematical methods or epidemiological terms are used:
Checks by code reviewer(s):