Refactor with LibmeshPetscCall(2)

[nmnobre]

Hi,

Would this be of interest? I can probably do the rest of the code in a morning if it would.

Cheers, -N

[moosebuild]

Job Test ARM mac on 3a156b6 : invalidated by @jwpeterson

Try to fix conda error "PackagesNotFoundError: moose-petsc"

[jwpeterson]

I assume that it was @lindsayad's intention to eventually replace all of the PETSc function call/error checking code with LibmeshPetscCall, so this looks like a good change to me, but I'll let him comment whether there is some reason to hold off on updating the remaining ones.

@roystgnr this PR was marked as a Draft on GitHub but the CI testing still ran. Is that the intended behavior in CIVET for Draft PRs? I seem to remember that marking a PR as Draft would prevent the tests from running before the user was ready to do so.

[nmnobre]

I assume that it was @lindsayad's intention to eventually replace all of the PETSc function call/error checking code with LibmeshPetscCall, so this looks like a good change to me, but I'll let him comment whether there is some reason to hold off on updating the remaining ones.

I think I've completed all replacements for the macros we already have. Unfortunately, I think we should have named LibmeshPetscCall(2), LibmeshPetscCallAbort(2) instead, so that LibmeshPetscCall would be free for idioms using CHKERRQ. As a workaround, I can create LibmeshPetscCallQ for the latter case and redefine LibmeshPetscCall2 to get an MPI communicator, so I can cover more cases. ~This way, when we think we're ready to drop support for PETSc < 3.17.0, we could simply:~ ~- redefine LibmeshPetscCall to wrap PetscCallAbort;~ ~- drop LibmeshPetscCall2 and call PetscCallAbort directly;~ ~- drop LibmeshPetscCall3 and call PetscCall directly.~ (I was so focused on the non-default code path, that I forgot by default we use PetscSolverException...)

@roystgnr this PR was marked as a Draft on GitHub but the CI testing still ran. Is that the intended behavior in CIVET for Draft PRs? I seem to remember that marking a PR as Draft would prevent the tests from running before the user was ready to do so.

FWIW, I actually find this very useful. :)

[roystgnr]

Running CI for Draft PRs is definitely the behavior I'd intend. I write ~~too much~~ occasional code that I want to run through the whole CI gauntlet even when I know there are other things that still need to be improved or even fixed before merging.

[roystgnr]

I'd rather not use PetscCall ever, personally. LIBMESH_CHKERR doesn't avoid using CHKERR not because the latter is a recent invention, but because throwing an exception that can either be caught or can trigger a terminate handler is strictly more useful than directly triggering a terminate handler.

[nmnobre]

I'd rather not use PetscCall ever, personally. LIBMESH_CHKERR doesn't avoid using CHKERR not because the latter is a recent invention, but because throwing an exception that can either be caught or can trigger a terminate handler is strictly more useful than directly triggering a terminate handler.

Yes, of course. Brain fart -.-, I was looking through the non-LIBMESH_ENABLE_EXCEPTIONS code path for a few hours to make sure I was calling the right macro, and completely forgot about the default code path. I think tomorrow I'll create LibmeshPetscCallQ which I think is a bit more descriptive than what I suggested earlier with LibmeshPetscCall3. (I think you guys might've named it LibmeshPetscCall2 because it takes 2 args).

[moosebuild]

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

Coverage

	7cda6a	#3973 fb0abb
	Total	Total	+/-	New
Rate	62.33%	62.32%	-0.01%	57.65%
Hits	72900	72531	-369	343
Misses	44065	43856	-209	252

Diff coverage report

Full coverage report

Warnings

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

This comment will be updated on new commits.

[nmnobre]

Done. All yours @roystgnr.

[lindsayad]

You can use [WIP] or WIP: in the PR title for disabling CIVET testing