aspect
aspect copied to clipboard
Update parameters.cc
Pull Request Checklist. Please read and check each box with an X. Delete any part not applicable. Ask on the forum if you need help with any step.
This pull request is meant to resolve #4496 by imposing a default upper bound on the rate of increase of the time step size.
Before your first pull request:
- [X] I have read the guidelines in our CONTRIBUTING.md document.
For all pull requests:
- [X] I have followed the instructions for indenting my code.
For new features/models or changes of existing features:
- [x] I have tested my new feature locally to ensure it is correct.
- [x] I have created a testcase for the new feature/benchmark in the tests/ directory.
- [x] I have added a changelog entry in the doc/modules/changes directory that will inform other users of my change.
here we're changing the default value of the maximum relative increase to 86 in order to limit the growth of the step size
Hi @laila-16 , thank you for your PR. Could you explain why you think the default value should be changed? Is it not enough to be able to change the maximum timestep increase from the input file? By default, we do not want to restrict the timestep size increase unnecessarily.
Hi @laila-16 , thank you for your PR. Could you explain why you think the default value should be changed? Is it not enough to be able to change the maximum timestep increase from the input file? By default, we do not want to restrict the timestep size increase unnecessarily.
according to the master thesis that was mentioned here https://github.com/geodynamics/aspect/issues/4496 , the growth of the time step should be limited by a factor of 1.86, changing the default value will ensure this, unless if the user change it from the input file.
Hi @laila-16, that would have been useful to mention when you open up the pull request. :-) We typically describe what we are doing and more importantly why we are doing it in the pull request description at the very top.
The change makes sense, but we will have to do more work for this to work:
- This number needs to be documented in the parameter description
- We should do some tests by running a few of the problems in cookbooks/benchmarks to see how big the influence is
- Tests will need updating
according to the master thesis that was mentioned here #4496 , the growth of the time step should be limited by a factor of 1.86, changing the default value will ensure this, unless if the user change it from the input file.
Thank you for the explanation and the link to the issue this PR addresses!
According to the issue, a factor of 1+sqrt(2) or 1.86 or 1.91 should be chosen. Why did you decide on 1.86?
according to the master thesis that was mentioned here #4496 , the growth of the time step should be limited by a factor of 1.86, changing the default value will ensure this, unless if the user change it from the input file.
Thank you for the explanation and the link to the issue this PR addresses!
According to the issue, a factor of 1+sqrt(2) or 1.86 or 1.91 should be chosen. Why did you decide on 1.86?
Hi @anne-glerum , sorry for the late reply. Here is the reason why I decided to choose 1.91 instead of 1.86 as I mentioned previously . The factor 1+sqrt(2) used for ODEs, while 1.86 and 1.91 used for PDEs. I looked into this a bit more, and the best available step-size ratio bound guaranteeing stability in the PDE context seems to be 1.91, from E. Emmrich (2006). Although that bound was proved in the context of semilinear parabolic problems, and doesn't strictly apply here, it seems like the best available point of reference and a reasonable default. I also ran a few tests comparing results with different values for this parameter and there was not a clear winner in terms of accuracy and efficiency.