-
Notifications
You must be signed in to change notification settings - Fork 8
Shaked/experimental examples #373
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
shakedregev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resetting CLI.
|
|
||
| ReSolve::LinSolverDirectKLU* KLU = new ReSolve::LinSolverDirectKLU; | ||
| ReSolve::LinSolverDirectCuSolverRf* Rf = new ReSolve::LinSolverDirectCuSolverRf; | ||
| ReSolve::LinSolverIterativeFGMRES* FGMRES = new ReSolve::LinSolverIterativeFGMRES(matrix_handler, vector_handler, GS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I understand why this is no longer using LinSolverIterativeFGMRES. Is it done under the hood? Should the file name be changed to reflect that we're testing something different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. It must've been overzealous copy-pasting. It was unintentional.
d768d3c to
5570b99
Compare
nkoukpaizan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Tests pass on Frontier and pdesx.
While testing, I noticed some warnings on r_KLU_rocsolverrf_asym6x6 that I missed in my review for a previous PR. Addressing those in #374.
Description
The experimental examples currently go through CSC storage. This is unnecessary and complicates the code from the machine and user perspective.
This is required for #357. It more completely addresses #362, which I originally interpreted as the redone examples only.
Proposed changes
Reworked everything with
setupCsrandgetFactorsCsr.Removed unnecessary usage of the Csc class.
Fixed some miscellaneous segfaults.
Simplified the logic for supplying the matrices and RHSs. There was some very weird and non-intuitive arithmetic going on there and I was not even sure how to call it.
This is how you call it currently, e.g. on pdesx for 3 systems:
On Frontier for 4 systems.:
Checklist
Put an
xin the boxes that apply. You can also fill these out after creatingthe PR. If you're unsure about any of them, don't hesitate to ask. We're here
to help! This is simply a reminder of what we are going to look for before
merging your code.
-Wall -Wpedantic -Wconversion -Wextra.