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

feat: Add option for automatic timestep computation in wave solvers #2909

Merged
merged 136 commits into from
Nov 1, 2024

Conversation

acitrain
Copy link
Contributor

@acitrain acitrain commented Dec 22, 2023

This PR add the possibility of compute automatically the timestep for wave propagation solvers.

How to use it?

  • You add the correct switch in your XML (timestepStabilityLimit=1)
  • You launch GEOS
  • If the requested dt for the solver (forceDt) is higher than the CFL condition limit, substeps are used to remain below the condition
    Also, an option is added to input source wavelets using TableFunctions, with the option sourceWaveletTableNames
    There also a routine inside the PR to be wrapped using Pygeosx

acitrain added 30 commits August 8, 2023 14:24
@acitrain acitrain added ci: run integrated tests Allows to run the integrated tests in GEOS CI and removed ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Oct 29, 2024
@@ -191,6 +185,18 @@ WaveSolverBase::WaveSolverBase( const std::string & name,
setSizedFromParent( 0 ).
setDescription( "Flag that indicates whether the receiver is local to this MPI rank" );

registerWrapper( viewKeyStruct::timestepStabilityLimitString(), &m_timestepStabilityLimit ).
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this always be used unless forceDt is set?

Copy link
Contributor Author

@acitrain acitrain Oct 31, 2024

Choose a reason for hiding this comment

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

In fact in this PR we are closer to the logic of the non-linear solvers: if you put a forceDt too big for the CFL condition the internal loop of the solver will do sub-steps to respect the CFL condition.
We decided to put this feature as an option to let the user be able to tests some dt on is own without getting into that routine.

@CusiniM
Copy link
Collaborator

CusiniM commented Oct 31, 2024

I have approved it but two things about this PR:

  1. please don't put PRs in the merge queue until they have all codeowners approvals.
  2. IMO, the idea of forceDt is for the timestep size to be fixed. To let the solver adjust the timestep size one can just set a maxTiimestep size for the event instead of forcing it. So, I would argue that, if the forceDt is too large it's fine for the simulation to fail instead of substepping. I know that solver base sort of does this too but we have talked about removing it coz it is actually a lot easier to let the EventManager be the only object managing timestepping.

@CusiniM CusiniM merged commit d19bdd7 into develop Nov 1, 2024
25 of 26 checks passed
@CusiniM CusiniM deleted the feature/AutomaticTimeStepComputationForWaveSolver branch November 1, 2024 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run code coverage enables running of the code coverage CI jobs ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants