Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation tests of numerical convergence #767

Merged
merged 39 commits into from
Jun 18, 2020

Conversation

glwagner
Copy link
Member

@glwagner glwagner commented May 29, 2020

This PR adds verification tests for numerical convergence of time-stepping and spatial discretization.

One test does not pass (the most complicated one): forced, fixed slip simulation. More detailed analysis of this test is needed.

Below is a summary of the results.

Time stepping convergence tests

image

Advection and diffusion of a one-dimensional cosine

cosine_advection_diffusion_solutions

cosine_advection_diffusion_error_convergence

Advection and diffusion of a one-dimensional Gaussian

image

image

Two-dimensional diffusion

image

Two-dimensional Taylor-Green vortex

taylor_green_convergence

Two-dimensional forced flow with free-slip boundary conditions

forced_free_slip_convergence

Two-dimensional forced flow with fixed-slip boundary conditions

This test does not pass

We don't know if there is a bug in the test or a problem in Oceananigans.

forced_fixed_slip_convergence

@codecov
Copy link

codecov bot commented May 29, 2020

Codecov Report

Merging #767 into master will increase coverage by 3.93%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #767      +/-   ##
==========================================
+ Coverage   70.97%   74.91%   +3.93%     
==========================================
  Files         124      124              
  Lines        2491     2603     +112     
==========================================
+ Hits         1768     1950     +182     
+ Misses        723      653      -70     
Impacted Files Coverage Δ
src/Forcing/simple_forcing.jl 81.81% <0.00%> (-18.19%) ⬇️
src/Models/incompressible_model.jl 88.88% <0.00%> (-11.12%) ⬇️
src/TimeSteppers/TimeSteppers.jl 66.66% <0.00%> (-8.34%) ⬇️
src/Utils/launch_config.jl 95.23% <0.00%> (-4.77%) ⬇️
...ntations/rozema_anisotropic_minimum_dissipation.jl 30.00% <0.00%> (-4.10%) ⬇️
src/Solvers/batched_tridiagonal_solver.jl 96.42% <0.00%> (-3.58%) ⬇️
src/Solvers/solve_for_pressure.jl 100.00% <0.00%> (ø)
src/OutputWriters/OutputWriters.jl 100.00% <0.00%> (ø)
src/OutputWriters/netcdf_output_writer.jl 84.90% <0.00%> (+0.29%) ⬆️
src/TurbulenceClosures/TurbulenceClosures.jl 37.93% <0.00%> (+1.08%) ⬆️
... and 14 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 e5c7aaf...d063105. Read the comment docs.

Copy link
Member

@ali-ramadhan ali-ramadhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome. Definitely a warm fuzzies™ PR! cc @sandreza

Resolves #611

@glwagner glwagner changed the title Verification tests of numerical convergence Validation tests of numerical convergence Jun 9, 2020
@glwagner
Copy link
Member Author

glwagner commented Jun 11, 2020

I have run the forced fixed slip test with multiple time-steps. The error does not decrease as the time-step decreases:

image

The results for different time-steps lie on top of one another.

@glwagner
Copy link
Member Author

glwagner commented Jun 18, 2020

At last, I found an algebra mistake in my definition of the forced fixed-slip problem. The forced fixed-slip convergence validation test now passes! This means that the Oceananigans pressure solver produces correct results when the pressure gradient is non-zero on the boundary.

image

@ali-ramadhan I think we should run these in CI eventually. Should we do that in this PR or save for a future PR?

@christophernhill
Copy link
Member

christophernhill commented Jun 18, 2020 via email

@ali-ramadhan
Copy link
Member

That's awesome! No one can make fun of us for not having convergence tests anymore!

@glwagner I might save it for a future PR just to get these scripts merged ASAP but we should do include them as tests before any numerical changes happen.

Would also be good to document all these tests in the "Verification experiments" section of the docs.

@sandreza
Copy link
Collaborator

@glwagner That's really awesome Greg!

@glwagner
Copy link
Member Author

Ok, I will merge this PR and open an issue to write up these tests in the docs, and also to execute the run scripts in tests to ensure they remain runnable as the code is changed.

We could also add one or more of the tests as a regression test. We can discuss in the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants