search

TDycores-Project / TDycore

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

Adds two-point flux as a new discretization #226

Closed bishtgautam closed 2 years ago

bishtgautam commented 2 years ago

Adds two-point flux as a separate discretization following the approach used in PFLOTRAN.

codecov-commenter commented 2 years ago

Codecov Report

Merging #226 (d5bdc73) into master (516e6a2) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #226   +/-   ##
=======================================
  Coverage   51.63%   51.63%           
=======================================
  Files           4        4           
  Lines         765      765           
=======================================
  Hits          395      395           
  Misses        370      370
Impacted Files Coverage Δ
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 dc57dd2...d5bdc73. Read the comment docs.

bishtgautam commented 2 years ago

@jeff-cohere This is ready for your review.

jeff-cohere commented 2 years ago

It feels like a lot of this code is going to be very, very temporary, since we're changing the way that we specify, track, and enforce boundary conditions. On the other hand, it's nice to have a proper two-point flux approximation.

The biggest question I have is whether it's worth adding all of the machinery for supporting integer-valued functions if we can get by without doing it (see my longest comment above). As far as I can tell, we can already determine the "BC type" for the entire domain using specified pressure/velocity/neither boundary conditions (or at least we could do so with modifications to make it clear that when you don't specify P or Vn, you're asking for a seepage condition).

What do you think, @bishtgautam ? Do you want to just add the integer-valued "BC type" functions for now? They'll go away as soon as the new BC code comes in, but if things are working now, maybe it's easiest to keep what's in this PR. Your call.

bishtgautam commented 2 years ago

I would like to keep the integer-value BC for now as I want to see how things will evolve when we have all BC-related info being read from a .exo file.

bishtgautam commented 2 years ago

I have now deleted the ComputeGeometry from MPFA-O and FV-TPF and replaced with TDyMeshComputeGeometry. (Though my head hurts because of all & * [] in the function)