TDycores-Project / TDycore

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

Added an interface for computing diagnostic fields #220

Closed jeff-cohere closed 2 years ago

jeff-cohere commented 2 years ago

This PR implements the solution to obtaining "diagnostic" fields computed from the solution vector, as discussed in #217. These functions are part of the TDycore interface and have their own implementation-specific function pointers. Currently, they are only implemented for the MPFA-O method.

See src/tdyio.c and demo/transient/transient_snes_mpfaof90.F90 for how these functions are used in practice.

Fixes #217

jeff-cohere commented 2 years ago

There's one remaining issue with this PR, which I've asked Jed about in #217

jeff-cohere commented 2 years ago

Okay, here's the updated version of our diagnostics field calculations. The interface consists of these functions:

@bishtgautam , I think you preferred to make TDyUpdateDiagnostics an internal function, but this requires us to decide when the diagnostics are updated automatically, and it's not clear to me that we know how to do this for all cases of interest. So can we keep it public for now and then tuck it inside if/when we decide it can be done automatically?

Also, saturation and liquid mass are the only diagnostic fields supported at the moment. I know TH needs a few more, but I think we can just add them very easily in another PR when we start to use them.

codecov-commenter commented 2 years ago

Codecov Report

Merging #220 (da6748a) into master (3cee685) will not change coverage. The diff coverage is n/a.

:exclamation: Current head da6748a differs from pull request most recent head c7db5f2. Consider uploading reports for the commit c7db5f2 to get more accurate results Impacted file tree graph

@@           Coverage Diff           @@
##           master     #220   +/-   ##
=======================================
  Coverage   51.63%   51.63%           
=======================================
  Files           4        4           
  Lines         765      765           
=======================================
  Hits          395      395           
  Misses        370      370           

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 3cee685...c7db5f2. Read the comment docs.

jeff-cohere commented 2 years ago

Hey @bishtgautam , have you had a chance to look at this yet? If the interface looks okay to you, we can get it merged.

bishtgautam commented 2 years ago

Apologies, this PR fell off my radar. I will look at it today.

bishtgautam commented 2 years ago

I only have one minor comment. Let me know if it is something you would like to address in this PR, otherwise, this is ready to be merged.