Closed jeff-cohere closed 3 years ago
@bishtgautam , can you add other people who you believe would have an opinion on this stuff? The FE folks will eventually care, I think, but this is still just a work in progress at the moment.
I like the logic also. One question that wasn't clear to me from a quick look over the changes: if TDySetDM is called, and then an option is passed in for a mesh (violating rulle 1), will Options throw an error?
I like the logic also. One question that wasn't clear to me from a quick look over the changes: if TDySetDM is called, and then an option is passed in for a mesh (violating rulle 1), will Options throw an error?
Yes, in the name of specificity. If this is too irritating and we'd rather use the option to override the DM supplied with TDySetDM, that's fine, but this is where I started. If you look at the changes in tdycore.c
, you can see how these rules are enforced in TDySetFromOptions
(and also TDySetDM
).
though, as I mention in a comment on this PR, I believe we should eliminate our own options and rely on the options that PETSc provides for generating DMPlexes, for simplicity and to leverage the work of the PETSc team
What is PETSc's option to read a mesh?
though, as I mention in a comment on this PR, I believe we should eliminate our own options and rely on the options that PETSc provides for generating DMPlexes, for simplicity and to leverage the work of the PETSc team
What is PETSc's option to read a mesh?
If we want to read a mesh, we can use our own -tdy_read_mesh
option. But if we specify -tdy_generate_mesh
, we can fall back on the DM and DMPlex-related options supported by DMSetFromOptions
(see PR #188 for a list)
So, instead of options in https://github.com/TDycores-Project/TDycore/blob/master/src/tdydm.c#L19-L53, use the following?
-dm_plex_box_dim <dim> - Set the topological dimension
-dm_plex_box_simplex <bool> - PETSC_TRUE for simplex elements, PETSC_FALSE for tensor elements
-dm_plex_box_lower <x,y,z> - Specify lower-left-bottom coordinates for the box
-dm_plex_box_upper <x,y,z> - Specify upper-right-top coordinates for the box
-dm_plex_box_faces <m,n,p> - Number of faces in each linear direction
-dm_plex_box_bd <bx,by,bz> - Specify the DMBoundaryType for each direction
-dm_plex_box_interpolate <bool> - PETSC_TRUE turns on topological interpolation (creating edges and faces)
So, instead of options in https://github.com/TDycores-Project/TDycore/blob/master/src/tdydm.c#L19-L53, use the following? ...
Yes, that's what I'm advocating. The benefit is that recent changes to PETSc (which we now have via the latest update) now make all these options available, and we could grab any new ones that appear without adding our own.
@jeff-cohere I agree with your proposal. Let's use PETSc options.
Cool. I'll get rid of the TDycore-specific mesh generation options and retrofit the demos to use the PETSc ones, and I'll let everyone know if I hit any snags.
@bishtgautam For reading in a mesh, you can use -dm_plex_filename
.
Merging #195 (8c5c533) into master (b7a410b) will increase coverage by
0.07%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #195 +/- ##
==========================================
+ Coverage 67.73% 67.81% +0.07%
==========================================
Files 7 7
Lines 1221 1224 +3
==========================================
+ Hits 827 830 +3
Misses 394 394
Impacted Files | Coverage Δ | |
---|---|---|
demo/mpfao/mpfao.c | 90.30% <100.00%> (+0.15%) |
:arrow_up: |
demo/richards/richards_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 b7a410b...8c5c533. Read the comment docs.
I don't think this covers all the ground mentioned in #188, but things are working, and I kinda don't want to make this PR much bigger. @bishtgautam , let me know if you'd like me to address anything else. @jedbrown , I made an issue for "vectorizing" our functions.
This PR gathers our command line options, elaborating on
TDyOptions
and adding features for specifying boundary condition functions and the like. I hope it's easier to understand.Also, I'm forcing us to be more specific about how we specify a mesh, because our rules to date have been confusing. Here are the new rules:
TDySetDM
is called beforeTDySetFromOptions
in a demo/driver, the DM passed to the dycore in that function is the mesh, period. It can't be overridden. We should only use this function in programs that use a fixed geometry, or that generate their own geometry without leaning onTDySetFromOptions
.TDySetDM
is not called beforeTDySetFromOptions
, the user must specify exactly one of-tdy_read_mesh <filename>
or-tdy_generate_mesh
. If-tdy_generate_mesh
is given, the properties of the mesh are extracted from our existing mesh-related command line options (though, as I mention in a comment on this PR, I believe we should eliminate our own options and rely on the options that PETSc provides for generating DMPlexes, for simplicity and to leverage the work of the PETSc team).TDySetFromOptions
returns successfully, TDycore has a distributed mesh with the right overlapping cells and all that--there's no need for anything else to be done.With these rules, it's always clear what the geometry is. This is good, because the word "obvious" is meaningless in every context in which anything matters. :-)
There's still work to be done. PR #188 proposes a lot of new options.