Is this a bug? MatMultAdd_SeqBAIJ_11
Hello, If we look at lines 2330-2331 in file baij2.c, it looks like there are some mistakes in assigning the `sum..` variables to the z array, causing the function MatMultAdd_SeqBAIJ_11() to not produce the correct answer. I don't have a good example program to demonstrate this yet - it's currently causing problems in a dev branch of pflotan_ogs that can produce blocksize 11 matrices. When in parallel, a standard matrix-vector multiplication calls MatMultAdd for the off-proc contributions, and the result is wrong when this is redirected to MatMultAdd_SeqBAIJ_11. Seems to be the root cause of several solvers failing such as fgmres. Can anyone confirm that these two lines seem incorrect? Thanks, Daniel
Hi Daniel,
On 21 Sep 2021, at 11:19, Daniel Stone <[email protected]> wrote:
Hello,
If we look at lines 2330-2331 in file baij2.c, it looks like there are some mistakes in assigning the `sum..` variables to the z array, causing the function MatMultAdd_SeqBAIJ_11() to not produce the correct answer.
I don't have a good example program to demonstrate this yet - it's currently causing problems in a dev branch of pflotan_ogs that can produce blocksize 11 matrices. When in parallel, a standard matrix-vector multiplication calls MatMultAdd for the off-proc contributions, and the result is wrong when this is redirected to MatMultAdd_SeqBAIJ_11. Seems to be the root cause of several solvers failing such as fgmres.
Can anyone confirm that these two lines seem incorrect?
Looks wrong to me, I guess this patch is correct? diff --git a/src/mat/impls/baij/seq/baij2.c b/src/mat/impls/baij/seq/baij2.c index 2849ef9051..65513c8989 100644 --- a/src/mat/impls/baij/seq/baij2.c +++ b/src/mat/impls/baij/seq/baij2.c @@ -2328,7 +2328,7 @@ PetscErrorCode MatMultAdd_SeqBAIJ_11(Mat A,Vec xx,Vec yy,Vec zz) v += 121; } z[0] = sum1; z[1] = sum2; z[2] = sum3; z[3] = sum4; z[4] = sum5; z[5] = sum6; z[6] = sum7; - z[7] = sum6; z[8] = sum7; z[9] = sum8; z[10] = sum9; z[11] = sum10; + z[7] = sum8; z[8] = sum9; z[9] = sum10; z[10] = sum11; if (!usecprow) { z += 11; y += 11; } Lawrence
I seem to have confirmed that making the change suggested by Lawrence fixes things in our case. Some alternate runs by a colleague that result in a blocksize 12 matrix also work fine - I think in that case MAtMultAdd_SeqBAIJ_N must be being used as I can't find a blocksize 12 analogue. Is there by any chance a setting somewhere that can tell petsc to override the use of blocksize specific routines like this and always go to, e.g., MatMultAdd_SeqBAIJ_N etc? Might be useful as a short term fix. Thanks, Daniel On Tue, Sep 21, 2021 at 11:50 AM Lawrence Mitchell <[email protected]> wrote:
Hi Daniel,
On 21 Sep 2021, at 11:19, Daniel Stone <[email protected]> wrote:
Hello,
If we look at lines 2330-2331 in file baij2.c, it looks like there are some mistakes in assigning the `sum..` variables to the z array, causing the function MatMultAdd_SeqBAIJ_11() to not produce the correct answer.
I don't have a good example program to demonstrate this yet - it's currently causing problems in a dev branch of pflotan_ogs that can produce blocksize 11 matrices. When in parallel, a standard matrix-vector multiplication calls MatMultAdd for the off-proc contributions, and the result is wrong when this is redirected to MatMultAdd_SeqBAIJ_11. Seems to be the root cause of several solvers failing such as fgmres.
Can anyone confirm that these two lines seem incorrect?
Looks wrong to me, I guess this patch is correct?
diff --git a/src/mat/impls/baij/seq/baij2.c b/src/mat/impls/baij/seq/baij2.c index 2849ef9051..65513c8989 100644 --- a/src/mat/impls/baij/seq/baij2.c +++ b/src/mat/impls/baij/seq/baij2.c @@ -2328,7 +2328,7 @@ PetscErrorCode MatMultAdd_SeqBAIJ_11(Mat A,Vec xx,Vec yy,Vec zz) v += 121; } z[0] = sum1; z[1] = sum2; z[2] = sum3; z[3] = sum4; z[4] = sum5; z[5] = sum6; z[6] = sum7; - z[7] = sum6; z[8] = sum7; z[9] = sum8; z[10] = sum9; z[11] = sum10; + z[7] = sum8; z[8] = sum9; z[9] = sum10; z[10] = sum11; if (!usecprow) { z += 11; y += 11; }
Lawrence
Daniel, Thanks for the update. We can patch the release version of PETSc (normally we don't have a way to patch previous release versions) There is no direct option to always use the _N version. (For smaller blocks using _N is very slow). Barry
On Sep 21, 2021, at 9:51 AM, Daniel Stone <[email protected]> wrote:
I seem to have confirmed that making the change suggested by Lawrence fixes things in our case. Some alternate runs by a colleague that result in a blocksize 12 matrix also work fine - I think in that case MAtMultAdd_SeqBAIJ_N must be being used as I can't find a blocksize 12 analogue.
Is there by any chance a setting somewhere that can tell petsc to override the use of blocksize specific routines like this and always go to, e.g., MatMultAdd_SeqBAIJ_N etc? Might be useful as a short term fix.
Thanks,
Daniel
On Tue, Sep 21, 2021 at 11:50 AM Lawrence Mitchell <[email protected] <mailto:[email protected]>> wrote: Hi Daniel,
On 21 Sep 2021, at 11:19, Daniel Stone <[email protected] <mailto:[email protected]>> wrote:
Hello,
If we look at lines 2330-2331 in file baij2.c, it looks like there are some mistakes in assigning the `sum..` variables to the z array, causing the function MatMultAdd_SeqBAIJ_11() to not produce the correct answer.
I don't have a good example program to demonstrate this yet - it's currently causing problems in a dev branch of pflotan_ogs that can produce blocksize 11 matrices. When in parallel, a standard matrix-vector multiplication calls MatMultAdd for the off-proc contributions, and the result is wrong when this is redirected to MatMultAdd_SeqBAIJ_11. Seems to be the root cause of several solvers failing such as fgmres.
Can anyone confirm that these two lines seem incorrect?
Looks wrong to me, I guess this patch is correct?
diff --git a/src/mat/impls/baij/seq/baij2.c b/src/mat/impls/baij/seq/baij2.c index 2849ef9051..65513c8989 100644 --- a/src/mat/impls/baij/seq/baij2.c +++ b/src/mat/impls/baij/seq/baij2.c @@ -2328,7 +2328,7 @@ PetscErrorCode MatMultAdd_SeqBAIJ_11(Mat A,Vec xx,Vec yy,Vec zz) v += 121; } z[0] = sum1; z[1] = sum2; z[2] = sum3; z[3] = sum4; z[4] = sum5; z[5] = sum6; z[6] = sum7; - z[7] = sum6; z[8] = sum7; z[9] = sum8; z[10] = sum9; z[11] = sum10; + z[7] = sum8; z[8] = sum9; z[9] = sum10; z[10] = sum11; if (!usecprow) { z += 11; y += 11; }
Lawrence
On Tue, Sep 21, 2021 at 10:04 AM Barry Smith <[email protected]> wrote:
Daniel,
Thanks for the update. We can patch the release version of PETSc (normally we don't have a way to patch previous release versions)
There is no direct option to always use the _N version. (For smaller blocks using _N is very slow).
It is in an MR: https://gitlab.com/petsc/petsc/-/merge_requests/4338 Thanks, Matt
Barry
On Sep 21, 2021, at 9:51 AM, Daniel Stone <[email protected]> wrote:
I seem to have confirmed that making the change suggested by Lawrence fixes things in our case. Some alternate runs by a colleague that result in a blocksize 12 matrix also work fine - I think in that case MAtMultAdd_SeqBAIJ_N must be being used as I can't find a blocksize 12 analogue.
Is there by any chance a setting somewhere that can tell petsc to override the use of blocksize specific routines like this and always go to, e.g., MatMultAdd_SeqBAIJ_N etc? Might be useful as a short term fix.
Thanks,
Daniel
On Tue, Sep 21, 2021 at 11:50 AM Lawrence Mitchell <[email protected]> wrote:
Hi Daniel,
On 21 Sep 2021, at 11:19, Daniel Stone <[email protected]> wrote:
Hello,
If we look at lines 2330-2331 in file baij2.c, it looks like there are some mistakes in assigning the `sum..` variables to the z array, causing the function MatMultAdd_SeqBAIJ_11() to not produce the correct answer.
I don't have a good example program to demonstrate this yet - it's currently causing problems in a dev branch of pflotan_ogs that can produce blocksize 11 matrices. When in parallel, a standard matrix-vector multiplication calls MatMultAdd for the off-proc contributions, and the result is wrong when this is redirected to MatMultAdd_SeqBAIJ_11. Seems to be the root cause of several solvers failing such as fgmres.
Can anyone confirm that these two lines seem incorrect?
Looks wrong to me, I guess this patch is correct?
diff --git a/src/mat/impls/baij/seq/baij2.c b/src/mat/impls/baij/seq/baij2.c index 2849ef9051..65513c8989 100644 --- a/src/mat/impls/baij/seq/baij2.c +++ b/src/mat/impls/baij/seq/baij2.c @@ -2328,7 +2328,7 @@ PetscErrorCode MatMultAdd_SeqBAIJ_11(Mat A,Vec xx,Vec yy,Vec zz) v += 121; } z[0] = sum1; z[1] = sum2; z[2] = sum3; z[3] = sum4; z[4] = sum5; z[5] = sum6; z[6] = sum7; - z[7] = sum6; z[8] = sum7; z[9] = sum8; z[10] = sum9; z[11] = sum10; + z[7] = sum8; z[8] = sum9; z[9] = sum10; z[10] = sum11; if (!usecprow) { z += 11; y += 11; }
Lawrence
-- What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead. -- Norbert Wiener https://www.cse.buffalo.edu/~knepley/ <http://www.cse.buffalo.edu/~knepley/>
participants (4)
-
Barry Smith -
Daniel Stone -
Lawrence Mitchell -
Matthew Knepley