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

Temporary workaround for MOOSE apps (mis)using LibmeshPetscCall #3984

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

nmnobre
Copy link
Member

@nmnobre nmnobre commented Oct 30, 2024

No description provided.

@moosebuild
Copy link

Job Coverage, step Generate coverage on b5e4298 wanted to post the following:

Coverage

Coverage did not change

Full coverage report

This comment will be updated on new commits.

@roystgnr
Copy link
Member

Copied to https://github.com/libMesh/libmesh/tree/farscape_workaround_test so we can test this in the MOOSE PRs. Please remind me to delete that when we're done with it.

Copy link
Member

@lindsayad lindsayad left a comment

Choose a reason for hiding this comment

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

Is this what just passed in idaholab/moose#28969?

@nmnobre
Copy link
Member Author

nmnobre commented Nov 14, 2024

Is this what just passed in idaholab/moose#28969?

Affirm.

@nmnobre nmnobre marked this pull request as ready for review November 14, 2024 18:31
@lindsayad
Copy link
Member

Such a simple process 😆

@lindsayad lindsayad merged commit 5435e2e into libMesh:devel Nov 14, 2024
20 checks passed
@jwpeterson
Copy link
Member

@nmnobre I think this PR caused a failure in the "Test No Exceptions" testing that we do for the devel -> master merge, see e.g. here. There are many error error messages from the compiler, but some representative ones are:

In file included from ./include/libmesh/petsc_shell_matrix.h:33,
                 from ./include/libmesh/petsc_matrix_shell_matrix.h:26,
                 from ./include/libmesh/static_condensation.h:26,
                 from ../src/base/dof_map.C:42:
./include/libmesh/petsc_vector.h: In constructor 'libMesh::PetscVector<T>::PetscVector(Vec, const libMesh::Parallel::Communicator&)':
./include/libmesh/petsc_vector.h:598:3: error: there are no arguments to 'LibmeshPetscCall' that depend on a template parameter, so a declaration of 'LibmeshPetscCall' must be available [-fpermissive]
   LibmeshPetscCall(VecGetLocalSize(_vec, &petsc_local_size));
   ^~~~~~~~~~~~~~~~
./include/libmesh/petsc_vector.h:598:3: note: (if you use '-fpermissive', G++ will accept your code, but allowing the use of an undeclared name is deprecated)

And many others along the same lines:

./include/libmesh/petsc_vector.h:602:3: error: there are no arguments to 'LibmeshPetscCall' that depend on a template parameter, so a declaration of 'LibmeshPetscCall' must be available [-fpermissive]
   LibmeshPetscCall(VecGetType(_vec, &ptype));
   ^~~~~~~~~~~~~~~~
./include/libmesh/petsc_vector.h:637:7: error: there are no arguments to 'LibmeshPetscCall' that depend on a template parameter, so a declaration of 'LibmeshPetscCall' must be available [-fpermissive]
       LibmeshPetscCall(VecGetLocalToGlobalMapping(_vec, &mapping));
       ^~~~~~~~~~~~~~~~

I see from your previous comment that reverting this PR is part of the 8 step plan so I guess the issue will eventually be resolved, but we won't have any devel -> master merges in the meantime.

@nmnobre
Copy link
Member Author

nmnobre commented Nov 18, 2024

The errors make sense, the temporary definition is only on the with-exceptions code path. I'm at SC24, I'll try to push a patch this afternoon (or feel free to go ahead with a patch).

@jwpeterson
Copy link
Member

It's not urgent for me (and I don't know exactly what the fix is!) so I'll let you take care of it when you have some time. Thanks!

nmnobre added a commit to farscape-project/libmesh that referenced this pull request Nov 18, 2024
nmnobre added a commit to farscape-project/libmesh that referenced this pull request Dec 16, 2024
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.

5 participants