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

Switch to using LibmeshPetscCall wrapper macros #28929

Draft
wants to merge 8 commits into
base: next
Choose a base branch
from

Conversation

nmnobre
Copy link
Member

@nmnobre nmnobre commented Oct 24, 2024

Hi @lindsayad,

Reason and Design

Same as that of libMesh/libmesh#3973.
Shortens the code and allows exception throwing.

I'm missing LIBMESH_CHKERR, I'll do that in another PR, I suspect this will be enough for a few CI problems already.

Cheers,
-N

nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 24, 2024
@nmnobre nmnobre marked this pull request as draft October 24, 2024 23:35
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 24, 2024
@@ -1 +1 @@
Subproject commit 5483644b60cc538a4b54d312f28835bd94107627
Subproject commit 09466ba6fa7df01e88e87f0c9d2945d103099440
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Precheck, step Conda libmesh build config check on 0e04814 wanted to post the following:

A change of the following file(s) triggered this check:

libmesh

The following file(s) are unchanged:

conda/mpi/conda_build_config.yaml
conda/libmesh/meta.yaml

The libmesh submodule or configuration was changed but the conda build config was not

@moosebuild
Copy link
Contributor

moosebuild commented Oct 25, 2024

Job Documentation, step Docs: sync website on 3bf0459 wanted to post the following:

View the site here

This comment will be updated on new commits.

@moosebuild
Copy link
Contributor

moosebuild commented Oct 25, 2024

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

Framework coverage

b8db75 #28929 3bf045
Total Total +/- New
Rate 85.08% 85.14% +0.07% 65.79%
Hits 106569 106248 -321 300
Misses 18694 18538 -156 156

Diff coverage report

Full coverage report

Modules coverage

Contact

b8db75 #28929 3bf045
Total Total +/- New
Rate 90.45% 90.50% +0.05% 85.11%
Hits 4936 4896 -40 40
Misses 521 514 -7 7

Diff coverage report

Full coverage report

External petsc solver

b8db75 #28929 3bf045
Total Total +/- New
Rate 86.25% 84.11% -2.14% 95.60%
Hits 389 307 -82 87
Misses 62 58 -4 4

Diff coverage report

Full coverage report

Navier stokes

b8db75 #28929 3bf045
Total Total +/- New
Rate 84.64% 84.62% -0.02% 97.67%
Hits 17081 17048 -33 42
Misses 3100 3098 -2 1

Diff coverage report

Full coverage report

Optimization

b8db75 #28929 3bf045
Total Total +/- New
Rate 88.54% 88.61% +0.06% 87.88%
Hits 1971 1921 -50 58
Misses 255 247 -8 8

Diff coverage report

Full coverage report

Stochastic tools

b8db75 #28929 3bf045
Total Total +/- New
Rate 90.67% 90.67% - 100.00%
Hits 8110 8110 - 21
Misses 835 835 - 0

Diff coverage report

Full coverage report

Full coverage reports

Reports

Warnings

  • framework new line coverage rate 65.79% is less than the suggested 90.0%
  • contact new line coverage rate 85.11% is less than the suggested 90.0%
  • optimization new line coverage rate 87.88% is less than the suggested 90.0%

This comment will be updated on new commits.

nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 25, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 25, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 25, 2024
@@ -1 +1 @@
Subproject commit 5483644b60cc538a4b54d312f28835bd94107627
Subproject commit 09466ba6fa7df01e88e87f0c9d2945d103099440
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Precheck, step Conda libmesh build config check on 2e92367 wanted to post the following:

A change of the following file(s) triggered this check:

libmesh

The following file(s) are unchanged:

conda/mpi/conda_build_config.yaml
conda/libmesh/meta.yaml

The libmesh submodule or configuration was changed but the conda build config was not

@lindsayad
Copy link
Member

@nmnobre please mention me when you take this off draft. Unsubscribing for now

@nmnobre
Copy link
Member Author

nmnobre commented Oct 25, 2024

@lindsayad No worries, won't happen until libMesh is updated anyway so... Can you please let me know why the apps tests are failing though?

@lindsayad
Copy link
Member

Errors like this

/data/civet2/build/pronghorn/subchannel/src/problems/InterWrapper1PhaseProblem.C: In function 'PetscErrorCode formFunctionIW(SNES, Vec, Vec, void*)':
/opt/libmesh/include/libmesh/petsc_solver_exception.h:143:21: error: invalid use of 'this' in non-member function
  143 |   LibmeshPetscCallA(this->comm().get(), __VA_ARGS__)
      |                     ^~~~
/opt/libmesh/include/libmesh/petsc_solver_exception.h:88:20: note: in definition of macro 'LIBMESH_CHKERRA'
   88 |     libmesh_ignore(comm);                       \
      |                    ^~~~
/opt/libmesh/include/libmesh/petsc_solver_exception.h:143:3: note: in expansion of macro 'LibmeshPetscCallA'
  143 |   LibmeshPetscCallA(this->comm().get(), __VA_ARGS__)
      |   ^~~~~~~~~~~~~~~~~
/data/civet2/build/pronghorn/subchannel/src/problems/InterWrapper1PhaseProblem.C:39:3: note: in expansion of macro 'LibmeshPetscCall'
   39 |   LibmeshPetscCall(VecGetSize(x, &size));
      |   ^~~~~~~~~~~~~~~~

@nmnobre
Copy link
Member Author

nmnobre commented Oct 26, 2024

Errors like this

/data/civet2/build/pronghorn/subchannel/src/problems/InterWrapper1PhaseProblem.C: In function 'PetscErrorCode formFunctionIW(SNES, Vec, Vec, void*)':
/opt/libmesh/include/libmesh/petsc_solver_exception.h:143:21: error: invalid use of 'this' in non-member function
  143 |   LibmeshPetscCallA(this->comm().get(), __VA_ARGS__)
      |                     ^~~~
/opt/libmesh/include/libmesh/petsc_solver_exception.h:88:20: note: in definition of macro 'LIBMESH_CHKERRA'
   88 |     libmesh_ignore(comm);                       \
      |                    ^~~~
/opt/libmesh/include/libmesh/petsc_solver_exception.h:143:3: note: in expansion of macro 'LibmeshPetscCallA'
  143 |   LibmeshPetscCallA(this->comm().get(), __VA_ARGS__)
      |   ^~~~~~~~~~~~~~~~~
/data/civet2/build/pronghorn/subchannel/src/problems/InterWrapper1PhaseProblem.C:39:3: note: in expansion of macro 'LibmeshPetscCall'
   39 |   LibmeshPetscCall(VecGetSize(x, &size));
      |   ^~~~~~~~~~~~~~~~

@lindsayad Right, well, those have nothing to do with this PR then, and will just break once libMesh is updated. That code is exploiting a loophole that used to exist that now doesn’t: it assumes libMesh was compiled with exceptions enabled, because if it weren’t, that would’ve already failed to compile even with the version of libMesh currently bundled with Moose.
LIBMESH_CHKERR (which LibmeshPetscCall used to call) didn’t use this->comm() with exceptions enabled, but did without exceptions… So, the question is, what’s the intended semantics of that code? CHKERRQ or CHKERRABORT? And can we change it?

@lindsayad
Copy link
Member

So, the question is, what’s the intended semantics of that code? CHKERRQ or CHKERRABORT? And can we change it?

It's likely a mix. Yes our team has access to subchannel if that's what you're asking

nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 26, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 26, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 26, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 26, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 26, 2024
@moosebuild
Copy link
Contributor

Job Precheck, step Conda libmesh build config check on 297cf96 wanted to post the following:

A change of the following file(s) triggered this check:

libmesh

The following file(s) are unchanged:

conda/mpi/conda_build_config.yaml
conda/libmesh/meta.yaml

The libmesh submodule or configuration was changed but the conda build config was not

@moosebuild
Copy link
Contributor

Job Coverage, step Verify coverage on 297cf96 wanted to post the following:

The following coverage requirement(s) failed:

  • external_petsc_solver coverage rate 84.11% is less than the required 85.0%

nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 29, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 29, 2024
@@ -1 +1 @@
Subproject commit 5483644b60cc538a4b54d312f28835bd94107627
Subproject commit 09466ba6fa7df01e88e87f0c9d2945d103099440
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Precheck, step Conda libmesh build config check on eaa43d8 wanted to post the following:

A change of the following file(s) triggered this check:

libmesh

The following file(s) are unchanged:

conda/mpi/conda_build_config.yaml
conda/libmesh/meta.yaml

The libmesh submodule or configuration was changed but the conda build config was not

@moosebuild
Copy link
Contributor

Job Coverage, step Verify coverage on eaa43d8 wanted to post the following:

The following coverage requirement(s) failed:

  • external_petsc_solver coverage rate 84.11% is less than the required 85.0%

nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 30, 2024
nmnobre added a commit to farscape-project/moose that referenced this pull request Oct 30, 2024
@@ -1 +1 @@
Subproject commit 5483644b60cc538a4b54d312f28835bd94107627
Subproject commit 09466ba6fa7df01e88e87f0c9d2945d103099440
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Precheck, step Conda libmesh build config check on 9994b5e wanted to post the following:

A change of the following file(s) triggered this check:

libmesh

The following file(s) are unchanged:

conda/mpi/conda_build_config.yaml
conda/libmesh/meta.yaml

The libmesh submodule or configuration was changed but the conda build config was not

@moosebuild
Copy link
Contributor

Job Precheck, step Clang format on 6126898 wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/28929/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format b8db753c73821a52d1624333d28c3c8bf9b2a008

@@ -1 +1 @@
Subproject commit 5483644b60cc538a4b54d312f28835bd94107627
Subproject commit 09466ba6fa7df01e88e87f0c9d2945d103099440
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Precheck, step Conda libmesh build config check on 6126898 wanted to post the following:

A change of the following file(s) triggered this check:

libmesh

The following file(s) are unchanged:

conda/mpi/conda_build_config.yaml
conda/libmesh/meta.yaml

The libmesh submodule or configuration was changed but the conda build config was not

@@ -1 +1 @@
Subproject commit 5483644b60cc538a4b54d312f28835bd94107627
Subproject commit 09466ba6fa7df01e88e87f0c9d2945d103099440
Copy link
Contributor

Choose a reason for hiding this comment

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

Caution! This contains a submodule update

@moosebuild
Copy link
Contributor

Job Precheck, step Conda libmesh build config check on 3bf0459 wanted to post the following:

A change of the following file(s) triggered this check:

libmesh

The following file(s) are unchanged:

conda/mpi/conda_build_config.yaml
conda/libmesh/meta.yaml

The libmesh submodule or configuration was changed but the conda build config was not

@moosebuild
Copy link
Contributor

Job Coverage, step Verify coverage on 3bf0459 wanted to post the following:

The following coverage requirement(s) failed:

  • external_petsc_solver coverage rate 84.11% is less than the required 85.0%

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.

3 participants