astro-turing / Integrating-diagenetic-equations-using-Python

Reactive-transport model simulating formation of limestone-marl alternations
Apache License 2.0
0 stars 1 forks source link

All Scenario and Solver parameters taken from config file, regression tests, increased modularity and cleanup. #15

Closed HannoSpreeuw closed 1 year ago

HannoSpreeuw commented 1 year ago

A lot more than just cleanup, i.e. a lot more than following up on the recommended suggestions from Pylint and Pylance:

1) I added two regression tests for Scenario A, one with Phi0=0.6 and PhiIni=0.5 (Figure 3e from L'Heureux) and one with Phi0=PhiIni=0.8. This required adding ground truth data, i.e. the output from the main branch (fixed porosity diffusion coefficent) for these parameters, as hdf5 files. I will incorporate these two regression tests into GH Actions. The former test takes 3 minutes while the latter takes 90 minutes, so that may be a bit long when a quick merge is requested. Also, the latter regression test currently fails, so I need to fix that before merge. I set a relative tolerance rtol=0.1 and an absolute tolerance atol=0.01. This should accommodate for any deviations near the surface due to the fact that a Dirichlet boundary condition can give rise to a boundary layer, see, e.g., paragraph 7.1 from Hundsdorfer: "Numerical Solution of Advection-Diffusion-Reaction Equations". 2) Renaming of the important Python modules. And I put these in a separate package: marlpde. Added a few empty __init__.py files in order to be able to call the modules. 3) So e.g. ScenarioA.py was renamed to Evolve_scenario.py. Within that module the functions integrate_equations and Plot_results have been separated out. This makes it more readable, but was actually a necessity to be able to implement the regression tests. 4) All Scenario parameters are now takes from the Scenario dataclass. This Python module was copied into this repo and I added a function that maps all the FORTRAN parameters onto the parameters used here, which, in turn, agree with the parameters used in the Matlab code. The way the solver parameters are set up in the Solver dataclassmarlpde.py has also been copied. 5) Plots now have the same format as in the Use_solve_ivp_without_py-pde_wrapper branch.

HannoSpreeuw commented 1 year ago

Fixed: the high porosity regression test passes now. b = 5 was divided by 1e4 twice. Merge kan proceed.