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

TDycore polymorphism, part 2: changing the DM lifecycle #214

Merged
merged 23 commits into from
Oct 25, 2021

Conversation

jeff-cohere
Copy link
Collaborator

@jeff-cohere jeff-cohere commented Oct 18, 2021

This is the second in a series of "surgical" pull requests to move TDycore to a polymorphic structure like the one used in PETSc (#197). The biggest change here is that we're getting rid of TDySetDM, which we've been using to construct a DM and hand it to TDycore to use.

Our new direction, as outlined in #211, is to reimagine the role of TDycore as a problem description from which a DM can be extracted and handed to a solver (KSP, SNES, TS, etc). In this picture:

  1. The user sets up the dycore with calls to TDySetMode, TDySetDiscretization (formerly TDySetDiscretizationMethod).
  2. Optionally, the user specifies a function/subroutine that the dycore can call to create a DM with TDySetDMConstructor.
  3. The dycore creates the DM during the TDySetFromOptions call.
  4. The user obtains the DM by calling TDyGetDM any time after TDySetFromOptions.
  5. The user fiddles with the DM if necessary (before or after calling TDySetup as needed).
  6. The DM is finalized by a call to TDySetup.
  7. The user passes the DM to an appropriate solver (not implemented in this PR).
  8. The solver handles the solution of the problem (not implemented in this PR).

Some may find it a bit jarring to have to define a function that creates a DM, instead of creating a DM and handing it to the dycore. The reason for this is that it actually simplifies the workflow--the dycore always controls when the DM is created, and has an opportunity to add boundary tags, distribute it, and so on.

Notes About Changes

  • An MPI communicator argument has been added to TDyCreate.
  • TDySetDiscretizationMethod has been renamed to TDySetDiscretization for brevity.
  • TDyMethod has been renamed to TDyDiscretization for specificity.
  • TDySetDM has been replaced with TDySetDMConstructor. If the latter name is too "C++-y", we can think of a better one. This function sets up a function pointer that is called by the dycore to create a DM.
  • The -tdy_generate_mesh command line argument has been removed. Any DM-related PETSc options passed to TDycore will override any settings specified in the DM constructor, and will define the attributes of the DM in the absence of a constructor.
  • TDySetup now calls a polymorphic function pointer for a given (mode, discretization).
  • Demos have been reworked to use TDySetDMConstructor instead of TDySetDM.
  • The TPF discretization has been removed.
  • TDyDriver does some things differently from the dycore itself. I think we should probably reorganize it so that it's layered on top of the dycore instead of doing things its own way. This is something we can do at the end of this polymorphism refactor.

Next Steps

After we merge this PR, I'm going to separate out all MPFA-O-specific data structures so they live in an MPFA-O-specific context that is manipulated by MPFA-O-specific function pointers. I think at that point, the purpose of this whole exercise will become much more clear--the TDy struct will only contain the essential elements of the problem description, plus a (mode, discretization)-specific context and a set of function pointers that use that context to perform implementation-specific operations.

@jeff-cohere jeff-cohere changed the title TDycore polymorphism, part 2 TDycore polymorphism, part 2: changing the DM lifecycle Oct 18, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2021

Codecov Report

Merging #214 (8ccbf05) into master (836f01b) will increase coverage by 0.29%.
The diff coverage is 54.16%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #214      +/-   ##
==========================================
+ Coverage   49.19%   49.48%   +0.29%     
==========================================
  Files           4        4              
  Lines         683      685       +2     
==========================================
+ Hits          336      339       +3     
+ Misses        347      346       -1     
Impacted Files Coverage Δ
demo/transient/transient.c 0.00% <0.00%> (ø)
demo/steady/steady.c 51.41% <55.55%> (+0.09%) ⬆️
demo/richards/richards_driver.c 100.00% <100.00%> (ø)
demo/th/th_driver.c 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 836f01b...8ccbf05. Read the comment docs.

@jeff-cohere jeff-cohere marked this pull request as ready for review October 18, 2021 18:38
@jeff-cohere jeff-cohere self-assigned this Oct 18, 2021
Copy link
Member

@bishtgautam bishtgautam left a comment

Choose a reason for hiding this comment

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

This is great work :+1 Looking forward to the next PR that cleans up MPFA-O stuff

@jeff-cohere
Copy link
Collaborator Author

@jedbrown , sorry for the poke (I'm sure you're pretty busy these days)! Do you want to weigh in on this one, or are you okay with TDycore hashing out the details as long as we end up using the DM as the source of truth in the way you're shooting for with libceed? No hurry if you want to take a look--I value your input.

@jeff-cohere
Copy link
Collaborator Author

@leorosie , I don't know how much these changes affect your work, but in case they do, ^^^

@jeff-cohere jeff-cohere merged commit b97463a into master Oct 25, 2021
@jeff-cohere jeff-cohere deleted the jeff-cohere/dm_setup branch October 25, 2021 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants