edoddridge / aronnax

An idealised isopycnal model that can be run either with n+1/2 layers, or with n layers and variable bathymetry.
http://aronnax.readthedocs.io/en/latest/
MIT License
23 stars 5 forks source link

Additional time stepping algorithms #184

Closed edoddridge closed 6 years ago

edoddridge commented 6 years ago

This PR refactors the code even further to separate the timestepping algorithm and allow for alternate algorithms to be implemented. As such it closes #182.

Furthermore, it implements a number of the timestepping algorithms discussed in #183. The Adams-Bashforth algorithms, Forward-Euler, and second-order Runge-Kutta are all implemented. However, only AB3 and RK2 are tested, since these were the original schemes for the main model loop and the initialisation respectively. Using the other algorithms causes the tests to fail, but the output fields look reasonable. This suggests the schemes are working as expected, since the outputs should be similar, but slightly different.

I would like to have a more intelligent test for the different schemes than "are they similar to output that was previously marked as correct". Constructing that test is a non-trivial science task, and is largely the reason for these new features. As such, I want the changes to be in master while I work on that. Happily, none of the changes in this PR alter the default behaviour of the model - unless one specifically sets TS_algorithm the model will perform exactly as it did before.

codecov[bot] commented 6 years ago

Codecov Report

Merging #184 into master will decrease coverage by 4.45%. The diff coverage is 64.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #184      +/-   ##
==========================================
- Coverage    95.4%   90.94%   -4.46%     
==========================================
  Files          17       19       +2     
  Lines        1327     1403      +76     
  Branches       74       76       +2     
==========================================
+ Hits         1266     1276      +10     
- Misses         43      107      +64     
- Partials       18       20       +2
Impacted Files Coverage Δ
src/advection_schemes.f90 100% <ø> (ø) :arrow_up:
src/thickness.f90 97.59% <ø> (ø) :arrow_up:
src/momentum.f90 100% <ø> (ø) :arrow_up:
src/barotropic_mode.f90 100% <ø> (ø) :arrow_up:
src/bernoulli.f90 100% <ø> (ø) :arrow_up:
src/end_run.f90 58.82% <ø> (ø) :arrow_up:
aronnax/driver.py 93.45% <ø> (ø) :arrow_up:
src/model_main.f90 93.96% <100%> (-1.98%) :arrow_down:
src/aronnax.f90 89.89% <100%> (+0.2%) :arrow_up:
src/adams_bashforth.f90 24.13% <24.13%> (ø)
... and 4 more

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 3a73413...680aa42. Read the comment docs.

edoddridge commented 6 years ago

As expected the diff coverage is abominable, since the new timestepping schemes are not being tested.

Nevertheless I'm keen to incorporate these changes into the repo, knowing that the default scheme is being tested, and that the default behaviour is unchanged.