Address review comments from PR #2539 - LU_SGS Preconditioner#2751
Address review comments from PR #2539 - LU_SGS Preconditioner#2751digvijay-y wants to merge 28 commits intosu2code:developfrom
Conversation
… GPU Preconditioner framework
… GPU Preconditioner framework
…ctions for indexing
…o edited the nvcc flags to match machine-specific architecture
…ted MVP, etc. LU_SGS Preconditioner Port
| void Reorder(vector<ScalarType>& pointList, vector<ScalarType>& inversePointList, vector<ScalarType> levelOffsets) { | ||
| void Reorder(const vector<ScalarType>& pointList, const vector<ScalarType>& levelOffsets, | ||
| vector<ScalarType>& reorderedPointList) { | ||
| auto levelOffsetsCursor = levelOffsets; |
There was a problem hiding this comment.
Reason for using a buffer variable? (Interesting variable name btw)
There was a problem hiding this comment.
Because of the moving variable, reorder increments per-level write positions, but original level offsets are kept unchanged for downstream
There was a problem hiding this comment.
I will have to re-read my code because I dont remember what I wrote a year ago. But won't this affect the calculations down the line? Did you find any difference in the values while comparing it to the CPU implementation?
There was a problem hiding this comment.
This looks good. Did you test this once with the flag on or off, just to make sure?
| void Partition(vector<ScalarType>& pointList, vector<ScalarType>& levelOffsets, vector<ScalarType>& chainPtr, | ||
| unsigned short rowsPerBlock) override { | ||
| vector<ScalarType> inversePointList; | ||
| inversePointList.reserve(nPointDomain); |
There was a problem hiding this comment.
This might cause issues although I'm not sure. Did the code run properly?
There was a problem hiding this comment.
I didn't run it completely! I'm on it.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| switch (KindAlgorithm) { | ||
| case ENUM_GRAPH_PART_ALGORITHM::LEVEL_SCHEDULING: | ||
| auto levelSchedule = CLevelScheduling<ScalarType>(nPointDomain, nodes); | ||
| levelSchedule.Partition(pointList, partitionOffsets, chainPtr, | ||
| LinearAlgebraUtils::ComputeRowsPerCudaBlock(config->GetCuda_Block_Size())); | ||
| nColor = levelSchedule.nLevels; | ||
| maxPartitionSize = levelSchedule.maxLevelWidth; | ||
| break; | ||
| } |
Check notice
Code scanning / CodeQL
No trivial switch statements Note
digvijay-y
force-pushed
the
gpu-lusgs
branch
2 times, most recently
from
29c9482 to
c3b8062
Compare
2 participants
Proposed Changes
Give a brief overview of your contribution here in a few sentences.
Changes request by @pcarruscag in PR #2539 made by @areenraj .
Related Work
Resolve any issues (bug fix or feature request), note any related PRs, or mention interactions with the work of others, if any.
#2539
PR Checklist
Put an X by all that apply. You can fill this out after submitting the PR. If you have any questions, don't hesitate to ask! We want to help. These are a guide for you to know what the reviewers will be looking for in your contribution.
pre-commit run --allto format old commits.