TDycores-Project / TDycore

BSD 2-Clause "Simplified" License
4 stars 0 forks source link

TDycore polymorphism, part 3: separating concerns #215

Closed jeff-cohere closed 2 years ago

jeff-cohere commented 2 years ago

Continuing in our journey (see #211 and #214):

This pull request defines interfaces for all important operations needed by our different numerical discretizations. Specifically:

In particular, there's a much stronger sense of an "interface" for TDycore. Instead of any piece doing whatever it wants at any point, we're moving toward a set of interfaces that perform specific tasks in specific contexts in a way that suggests a flow of information that is easier to understand.

So where are we?

Everything works.

What's next?

The bad news is: not everything will be perfect after this PR goes in. A lot of cleanup remains, but much of it is within the separate numerical implementations, which lets different parts of our team adjust things without affecting others.

The good news: the changes in this PR make it easier for us to move in the direction of the DM-oriented problem definition that we started discussing in #211. Briefly: we attach functions to a DM for computing residuals, Jacobians, etc, and then we hand the DM over to the solvers. This allows us to make TDycore's interface smaller and tighter--the dycore produces a DM that gets used by the solvers directly.

Fixes #133 Fixes #196 Fixes #197 Fixes #22

jeff-cohere commented 2 years ago

UPDATE: I've fixed the transient_mpfaof90 demos and the transient_snes_mpfaof90 demos, aside from the one that reads in the mesh geometry data from a file (which I'm not convinced is working correctly, and which we might want to disable for the purposes of this PR).

The remaining tests are related to the TH model and the Richards driver. That sounds like a post-holiday adventure.

jeff-cohere commented 2 years ago

Okay, I've fixed some issues I found in the higher-level driver interface, which drives the richards and th demos. Now these drivers run, but they get bad answers. I could go ahead and find out exactly what's going on, but it would be faster if someone who knew more about the driver interface could help me understand the assumptions that it makes.

These demos and the transfer of saved geometry info in the transient_snes_mpfaof90 demo are the only issues remaining, as far as I can tell. @bishtgautam , maybe we should discuss how to proceed.

jeff-cohere commented 2 years ago

The Richards driver tests now pass. The TH ones still fail.

Also, I found the source of the issue with reading in mesh geometry from a binary file: the new mesh construction process has already converted indices to compressed row format by the time the data is read, whereas the old construction process read in the geometry before doing that conversion.

jeff-cohere commented 2 years ago

@bishtgautam and I agreed that we can comment out the mesh geometry reading/writing stuff for now, as we may not actually need it.

The TH tests are the only remaining failures.

codecov-commenter commented 2 years ago

Codecov Report

Merging #215 (fe06c7a) into master (b97463a) will increase coverage by 2.14%. The diff coverage is 67.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #215      +/-   ##
==========================================
+ Coverage   49.48%   51.63%   +2.14%     
==========================================
  Files           4        4              
  Lines         685      765      +80     
==========================================
+ Hits          339      395      +56     
- Misses        346      370      +24     
Impacted Files Coverage Δ
demo/transient/transient.c 0.00% <0.00%> (ø)
demo/steady/steady.c 54.13% <72.20%> (+2.72%) :arrow_up:
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 b97463a...fe06c7a. Read the comment docs.