Skip to content

Conversation

stefanozampini
Copy link
Contributor

Description

There is no need to maintain obsolete CHKERR and SETERR implementations in Firedrake.

PETSc management of tracebacks from Cython/Python was improved one year ago in https://gitlab.com/petsc/petsc/-/merge_requests/7938, and now we can get valuable information from catching errors.

Needs CHKERRMPI exposed in https://gitlab.com/petsc/petsc/-/merge_requests/8758

For example, adding mixing INSERT and ADD values will trigger an error in the PETSc C code that will be forwarded appropriately (through PETSC_ERR_PYTHON) to the Python runtime (the patch is for the code in this PR)

diff --git a/pyop2/sparsity.pyx b/pyop2/sparsity.pyx
index 5914bacc8..f4dcd4d8f 100644
--- a/pyop2/sparsity.pyx
+++ b/pyop2/sparsity.pyx
@@ -50,6 +50,7 @@ cdef extern from "petsc.h":
         PETSC_TRUE, PETSC_FALSE
     ctypedef enum PetscInsertMode "InsertMode":
         PETSC_INSERT_VALUES "INSERT_VALUES"
+        PETSC_ADD_VALUES "ADD_VALUES"
     ctypedef enum PetscErrorCode:
         PETSC_SUCCESS

@@ -200,6 +201,7 @@ def fill_with_zeros(PETSc.Mat mat not None, dims, maps, iteration_regions, set_d
         for i in range(nrow // rdim):
             if i < ncol  // cdim:
                 CHKERR(MatSetValuesBlockedLocal(mat.mat, 1, &i, 1, &i, diag_values, PETSC_INSERT_VALUES))
+                CHKERR(MatSetValuesBlockedLocal(mat.mat, 1, &i, 1, &i, diag_values, PETSC_ADD_VALUES))
         CHKERR(PetscFree(diag_values))
     if len(maps) == 0:
         return

With the current firedrake code, we get

pyop2/sparsity.pyx:163: in pyop2.sparsity.build_sparsity
    fill_with_zeros(preallocator, (1, 1),
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

>   CHKERR(MatSetValuesBlockedLocal(mat.mat, 1, &i, 1, &i, diag_values, PETSC_ADD_VALUES))
E   RuntimeError: 73

pyop2/sparsity.pyx:220: RuntimeError

while with the code in this PR

pyop2/sparsity.pyx:147: in pyop2.sparsity.build_sparsity
    fill_with_zeros(preallocator, (1, 1),
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

>   CHKERR(MatSetValuesBlockedLocal(mat.mat, 1, &i, 1, &i, diag_values, PETSC_ADD_VALUES))
E   petsc4py.PETSc.Error: error code 73
E   [0] MatSetValuesBlockedLocal() at /Users/zampins/Devel/petsc/src/mat/interface/matrix.c:2585
E   [0] Object is in wrong state
E   [0] Cannot mix add values and insert values

pyop2/sparsity.pyx:204: Error

@stefanozampini stefanozampini requested a review from dham October 2, 2025 12:48
@stefanozampini stefanozampini self-assigned this Oct 2, 2025
Copy link
Contributor

@connorjward connorjward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great. Thanks!

Note that this won't be merge-able until PETSc release 3.24.1 because our release branch follows released PETSc versions.

@stefanozampini
Copy link
Contributor Author

stefanozampini commented Oct 3, 2025 via email

@stefanozampini
Copy link
Contributor Author

@connorjward Tests pass

@connorjward connorjward merged commit ae809f3 into main Oct 9, 2025
11 of 14 checks passed
@connorjward connorjward deleted the stefanozampini/chkerr-from-petsc4py branch October 9, 2025 09:19
@connorjward
Copy link
Contributor

@stefanozampini thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants