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

PetscFE determines basis for examples/fluids/navierstokes #1264

Merged
merged 15 commits into from
Aug 2, 2023
Merged

Conversation

KennethEJansen
Copy link
Collaborator

As we prepare to bring simplex meshes into the examples/fluids/navierstokes application within libCEED, it is important to let PETSc define the basis functions. This code revision followed similar work done in Ratel to address this issue.

This has been completed at this point for the primary operators but not for the Data/Learning extensions but hopefully those will be resolved within this PR.

As all changes are within examples/fluids and off of a current main, all tests of libCEED should still pass.

As the work mimicked Ratel development (and work from a branch of Rezgar Shekeri) to acquire the basis information from PETSc, credit should go those efforts while responsibility for miss-interpretations of that effort belong to Kenneth Jansen with co-author help from James Wright.

At this time the fluids example has been verified correct without loss of computational efficiency on Polaris in both serial and parallel including STG with Q1, Q2, and Q3 elements. Further tests with simplex elements are ongoing and updates will be provided as they become available.

@jrwrigh
Copy link
Collaborator

jrwrigh commented Jul 25, 2023

TODO:

  • Remove Q_dim from GetRestrictionForDomain arguments (just use Q for quadrature points per element, aligned with CeedBasis nomenclature)
  • Style/rearrange
  • Fix all calls to GetRestrictionForDomain
  • Fix test errors.

@KennethEJansen
Copy link
Collaborator Author

In the failing tests I see -q_extra 2 I guess it is obvious that PestscFE does not understand/honor this request.

@jedbrown how do you propose we resolve this? Is there a way to communicate this request to PETSc?

@jrwrigh
Copy link
Collaborator

jrwrigh commented Jul 25, 2023

In the failing tests I see -q_extra 2 I guess it is obvious that PestscFE does not understand/honor this request.

@jedbrown how do you propose we resolve this? Is there a way to communicate this request to PETSc?

I'm guessing we probably just need to change:

PetscCall(PetscFECreateLagrange(PETSC_COMM_SELF, problem->dim, num_comp_q, is_simplex, degree, PETSC_DECIDE, &fe));

from PETSC_DECIDE to the actual quadrature order we want. See PETSc docs

@jedbrown
Copy link
Member

@jrwrigh jrwrigh mentioned this pull request Jul 26, 2023
8 tasks
KennethEJansen and others added 13 commits July 31, 2023 17:21
- Small, insignificant changes to the coordinate locations make a
  dramatic difference to the initial condition
- For the test case, there is a node right at the midpoint of the
  domain. Adding the epsilon fudge factor makes the if statement more
  stable
 - Change the test so that it passes on all hardware. It's a very
   sentitive test and the changes to the basis creation changed the
   initial conditions *just* enough to make the test not happy. Should
   really try and make this more robust at some point...
@jrwrigh jrwrigh merged commit 0814d5a into main Aug 2, 2023
26 checks passed
@jrwrigh jrwrigh deleted the SYCL-PetscFE branch August 2, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants