Skip to content
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

Allow all Petsc matrix types to proceed from a common base class #3914

Merged
merged 10 commits into from
Aug 13, 2024

Conversation

lindsayad
Copy link
Member

And critically, always allow retrieving a PetscMatrix context from a PETSc mat. Eventually we may be able to remove the PetscMatrix from Mat constructors entirely. That would be good

This change was strongly suggested by the need to convert LinearSolver to be able to handle StaticCondensation

And critically, always allow retrieving a PetscMatrix context
from a PETSc mat. Eventually we may be able to remove the
PetscMatrix from Mat constructors entirely. That would be good

This change was strongly suggested by the need to convert LinearSolver
to be able to handle StaticCondensation
@moosebuild
Copy link

moosebuild commented Aug 1, 2024

Job Coverage on 84661f7 wanted to post the following:

Coverage

24bb5a #3914 84661f
Total Total +/- New
Rate 63.29% 63.28% -0.01% 66.76%
Hits 71626 71667 +41 239
Misses 41544 41590 +46 119

Diff coverage report

Full coverage report

Warnings

  • New new line coverage rate 66.76% is less than the suggested 90.0%

This comment will be updated on new commits.

@lindsayad lindsayad marked this pull request as ready for review August 1, 2024 20:59
include/numerics/sparse_matrix.h Outdated Show resolved Hide resolved
include/numerics/sparse_matrix.h Outdated Show resolved Hide resolved
src/numerics/shell_matrix.C Outdated Show resolved Hide resolved
src/solvers/petsc_linear_solver.C Outdated Show resolved Hide resolved
src/solvers/petsc_nonlinear_solver.C Outdated Show resolved Hide resolved
src/numerics/petsc_matrix.C Outdated Show resolved Hide resolved
src/numerics/petsc_matrix.C Outdated Show resolved Hide resolved
src/numerics/petsc_matrix.C Outdated Show resolved Hide resolved
src/numerics/petsc_matrix.C Outdated Show resolved Hide resolved
src/numerics/petsc_matrix.C Outdated Show resolved Hide resolved
@moosebuild
Copy link

Job Test MOOSE clang on 84661f7 : invalidated by @lindsayad

@moosebuild
Copy link

Job Test MOOSE GCC min on 84661f7 : invalidated by @lindsayad

@moosebuild
Copy link

Job Test MOOSE debug on 84661f7 : invalidated by @lindsayad

@moosebuild
Copy link

Job Test MOOSE min clang on 84661f7 : invalidated by @lindsayad

@lindsayad lindsayad merged commit f29d030 into libMesh:devel Aug 13, 2024
20 checks passed
@lindsayad lindsayad deleted the petsc-matrix-base branch August 13, 2024 21:19
@jwpeterson
Copy link
Member

This PR caused a build failure on the libmesh devel -> master merge for the --disable-exceptions build.

In file included from /opt/petsc/include/petscsys.h:1100,
                 from /opt/petsc/include/petscbag.h:3,
                 from /opt/petsc/include/petsc.h:6,
                 from ./include/libmesh/petsc_macro.h:56,
                 from ./include/libmesh/petsc_matrix_base.h:29,
                 from ../src/numerics/petsc_matrix_base.C:25:
../src/numerics/petsc_matrix_base.C: In static member function 'static libMesh::PetscMatrixBase<T>* libMesh::PetscMatrixBase<T>::get_context(Mat)':
./include/libmesh/petsc_solver_exception.h:96:42: error: 'this' is unavailable for static member functions
 #define LIBMESH_CHKERR(ierr) CHKERRABORT(this->comm().get(), ierr);
                                          ^~~~
/opt/petsc/include/petscerror.h:713:25: note: in definition of macro 'PetscCallAbort'
         (void)MPI_Abort(comm, (PetscMPIInt)ierr_petsc_call_abort_); \
                         ^~~~
./include/libmesh/petsc_solver_exception.h:96:30: note: in expansion of macro 'CHKERRABORT'
 #define LIBMESH_CHKERR(ierr) CHKERRABORT(this->comm().get(), ierr);
                              ^~~~~~~~~~~
./include/libmesh/petsc_solver_exception.h:129:5: note: in expansion of macro 'LIBMESH_CHKERR'
     LIBMESH_CHKERR(libmesh_petsc_call_ierr);                                                       \
     ^~~~~~~~~~~~~~
../src/numerics/petsc_matrix_base.C:119:3: note: in expansion of macro 'LibmeshPetscCall'
   LibmeshPetscCall(PetscObjectQuery((PetscObject)mat, "PetscMatrixCtx", (PetscObject *)&container));
   ^~~~~~~~~~~~~~~~
./include/libmesh/petsc_solver_exception.h:96:42: error: 'this' is unavailable for static member functions
 #define LIBMESH_CHKERR(ierr) CHKERRABORT(this->comm().get(), ierr);
                                          ^~~~
/opt/petsc/include/petscerror.h:713:25: note: in definition of macro 'PetscCallAbort'
         (void)MPI_Abort(comm, (PetscMPIInt)ierr_petsc_call_abort_); \
                         ^~~~
./include/libmesh/petsc_solver_exception.h:96:30: note: in expansion of macro 'CHKERRABORT'
 #define LIBMESH_CHKERR(ierr) CHKERRABORT(this->comm().get(), ierr);
                              ^~~~~~~~~~~
./include/libmesh/petsc_solver_exception.h:129:5: note: in expansion of macro 'LIBMESH_CHKERR'
     LIBMESH_CHKERR(libmesh_petsc_call_ierr);                                                       \
     ^~~~~~~~~~~~~~
../src/numerics/petsc_matrix_base.C:123:3: note: in expansion of macro 'LibmeshPetscCall'
   LibmeshPetscCall(PetscContainerGetPointer(container, &ctx));
   ^~~~~~~~~~~~~~~~

@lindsayad
Copy link
Member Author

beautiful failed code

@jwpeterson
Copy link
Member

jwpeterson commented Aug 14, 2024

Yeah, the LIBMESH_CHKERR macro uses this when exceptions are disabled, so it can't be used in static functions, which I guess means that LibmeshPetscCall can't be called in static functions...

Maybe we could add a version of these exceptionless macros that calls CHKERRABORT with the communicator from the Mat that is passed in to get_context()?

@lindsayad
Copy link
Member Author

it could be good to have such a macro in the future. For now, I think we have a good change in #3925

lindsayad added a commit to lindsayad/libmesh that referenced this pull request Aug 18, 2024
With the change in libMesh#3914 to make the `_mat` data member the same
as the `PetscMatrix(Base)` class, the manual destruction is now
required. This refs the valgrind failures on idaholab/moose#28411
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.

4 participants