-
Notifications
You must be signed in to change notification settings - Fork 578
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
Amesos2: Fix single-proc case for non-contiguous GIDs #4454
Conversation
Potential fix for issue #3647
@vbrunini this PR fixed the single proc case with KLU2 I used while working on the issue, labeled WIP until I cleanup the test. If you'd like to try it before merge please pull this PR; your code will require setting a parameter in the parameter list to indicate to Amesos2 that the row ids are "gapped" or non-contiguous, here is sample syntax:
|
@vbrunini here's some updates on this PR: In the recent commit I pushed an example A couple comments about the example: To run with non-default matrices, you can execute something like this: The example is hard-coded to detect "badMatrix" in the By default a parameter list is created for the solver that sets "IsContiguous" to "false", indicating to Amesos2 that the matrix contains gaps in the row ids and to accommodate for this. This can be disabled with the command line argument
It looks like it's important the I tested this example with KLU2 and SuperLU 4.3 using the badMatrix etc. files and I get correct results (within tolerance via residual check); I had issues building SuperLU_DIST TPL locally and need to test on a machine with SEMS modules next. Prior to this example, I wrote some separate driver code to read the badMatrix, badRhs and rowMap files into corresponding Tpetra data structures, convert them to have locally contiguous GIDs and then write to matrix market files. I was then able to read those files into Octave and compute a solution as a sanity check that the matrix itself was solvable, and the results also agreed well with the solvers used in Amesos2. If it's useful I can clean up the driver codes or files and send them to you. Hopefully the updates from this PR allow your code to work with KLU2 or SuperLU (I'll post updates once I finish testing with SLUDIST); if the changes here don't resolve the issues you are seeing it may help if we can follow up offline to figure out what extra info beyond the data files I'll need to reproduce and resolve. |
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
Using Repos:
Pull Request Author: ndellingwood |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_cuda_9.2
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
@srajama1 if you get a break during the conference can you review? |
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 thought this change doesn't fix the problem originally reported. I assume this is an unrelated problem you would like to be fixed anyway, so you are taking care of it. If yes, please go ahead.
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ srajama1 ]! |
Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged |
Merge on Pull Request# 4454: IS A SUCCESS - Pull Request successfully merged |
@srajama1 it fixes the problem for the matrix, rhs, and map provided. |
Potential fix for issue #3647