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

Add Backward Euler solver (rebased) #2239

Merged
merged 9 commits into from
Apr 8, 2021
Merged

Add Backward Euler solver (rebased) #2239

merged 9 commits into from
Apr 8, 2021

Conversation

bendudson
Copy link
Contributor

Replaces #2238, based on an updated next branch to avoid conflicts.

Convert the SNES solver (which never converged for real problems)
into a backward Euler solver. Both `beuler` and `snes` solver type
names can be used for now.

Nearly twice as fast as CVODE for same atol/rtol settings (10 mins vs
19) on quite complex 1D Hermes simulation with neon
(https://github.com/bendudson/hermes-3/tree/master/examples/1D-neon)
Explicit comparisons against zero error code
Out of domain errors should be handled by calling PETSc function,
rather than returning an error code.
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 33. Check the log or trigger a new build to see more.

src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
src/solver/impls/snes/snes.cxx Outdated Show resolved Hide resolved
* next:
  Remove the Karniadakis time solver
  clang-tidy suggetion
  Manual entry on provenance-tracking metadata
  Save provenance tracking info from grid file
@ZedThree
Copy link
Member

Fiddling about this to try and get it to pass test-solver, it seems the biggest effect comes from setting the output timestep. Adding the following to test_solver.cxx is "sufficient" to get it to pass, but then it does take ~35 seconds:

  constexpr int beuler_nout = 1e5;
  root["beuler"]["nout"] = beuler_nout;
  root["beuler"]["output_step"] = end / beuler_nout;

Making the initial timestep very small, or tightening the tolerances makes very little difference.

This is really a steady-state solver, and not suitable for oscillatory
solutions like the one in this test
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -47,39 +47,55 @@ class SNESSolver;

namespace {
RegisterSolver<SNESSolver> registersolversnes("snes");
}
RegisterSolver<SNESSolver> registersolverbeuler("beuler");
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: variable registersolverbeuler defined in a header file; variable definitions in header files can lead to ODR violations [misc-definitions-in-headers]

RegisterSolver<SNESSolver> registersolverbeuler("beuler");
                           ^

public:
SNESSolver(Options *opt = nullptr) : Solver(opt) {}
public:
SNESSolver(Options* opt = nullptr) : Solver(opt) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: constructor does not initialize these fields: timestep, dt, lower_its, upper_its, out_timestep, nsteps, diagnose, nlocal, neq, snes_f, snes_x, x0, predictor, x1, snes, Jmf [cppcoreguidelines-pro-type-member-init]

SNESSolver(Options* opt = nullptr) : Solver(opt) {}
^

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

tests/integrated/test-beuler/test_beuler.cxx Show resolved Hide resolved
tests/integrated/test-beuler/test_beuler.cxx Show resolved Hide resolved
tests/integrated/test-beuler/test_beuler.cxx Show resolved Hide resolved
// "solver" section, as we run into problems when solvers use the same
// name for an option with inconsistent defaults
auto options = Options::getRoot()->getSection("beuler");
auto solver = std::unique_ptr<Solver>{Solver::create("beuler", options)};
Copy link
Contributor

Choose a reason for hiding this comment

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

warning: auto options can be declared as auto *options [readability-qualified-auto]

auto options = Options::getRoot()->getSection("beuler");
^~~~~
auto *

bendudson and others added 2 commits March 19, 2021 13:35
Evolves to steady state, compares against known solution.
Co-authored-by: Peter Hill <zed.three@gmail.com>
@ZedThree ZedThree merged commit f4a5e42 into next Apr 8, 2021
@ZedThree ZedThree deleted the next-snes-solver2 branch April 8, 2021 16:09
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

3 participants