Repository navigation
Fix SemiImplicitBDF start-up order off-by-one - #355
Open
NopeCharon wants to merge 1 commit into
Open
NopeCharon wants to merge 1 commit into
NopeCharon wants to merge 1 commit into
Conversation
SemiImplicitBDF ramps up from first order to the full order-k BDF/EXT
scheme while the history of previous solutions is filled, but step()
indexed its start-up solvers off by one:
if (self.k == 1) or (iter > self.k - 1):
self.petsc_solver.solve()
else:
self.startup_solvers[iter - 1].solve()
startup_solvers[j] is the order-(j+1) scheme, so iter = 0 wrapped
around to startup_solvers[-1] (the highest start-up order), iter = 1
fell back to first order, and the full scheme only took over at
iter = k. With the default order=3 the first steps ran with orders
2, 1, 2, 3 instead of 1, 2, 3, 3; with order=2 they ran 1, 1, 2.
At step iter the history holds iter + 1 solutions, so use the order
min(iter + 1, k) scheme: the full solver once iter >= k - 1 and
startup_solvers[iter] before that. This also covers k == 1 without the
special case added in dynamicslab#189.
For a developed Re=100 cylinder flow integrated to t = 0.1 from a
fresh solver, the velocity error against a dt = 1e-3 reference drops
from 2.06e-3 to 1.09e-4 at dt = 1e-2 and from 9.05e-4 to 2.65e-5 at
dt = 5e-3.
Add test_bdf_startup_order to test/test_cyl.py, which records the order
of the scheme used at each step for k = 1, 2, 3.
Fixes dynamicslab#354
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #354
SemiImplicitBDF.step()indexed its start-up solvers off by one.startup_solvers[j]is the order-j+1scheme, but stepiterusedstartup_solvers[iter - 1]. Soiter = 0wrapped around to the highest start-up order,iter = 1fell back to first order, and the full scheme only took over atiter = k:orderChanges
iterthe history holdsiter + 1solutions. The solver now uses the full scheme onceiter >= k - 1andstartup_solvers[iter]before that, i.e. ordermin(iter + 1, k). This also coversk == 1, so the special case from Fix indexing error in BDF with k=1 #189 is no longer needed.test_bdf_startup_order(intest/test_cyl.py) records which scheme order runs at each step fork = 1, 2, 3.Validation
All runs used the
firedrakeproject/firedrake-vanilla-defaultcontainer.main,test_bdf_startup_order[2]and[3]fail ([1, 1, 2]and[2, 1, 2, 3]). With this change all three pass.test_integrate[1],test_integrate[3],test_envandtest_controlintest/test_cyl.pypass.test_steady_rotationfails onmaintoo: CL ≈ -0.46 against the expected -0.06, with or without this change. This PR doesn't touch it.order=3from a fresh solver. Velocity error (fd.errornorm) against a dt = 1e-3 reference:mainruff check,ruff format --check,isort --check-onlyandcodespellpass.