search

dougshidong / PHiLiP

Parallel High-Order Library for PDEs through hp-adaptive Discontinuous Galerkin methods
Other
45 stars 36 forks source link

Explicit ODE Solver Restructure #105

Closed cpethrick closed 2 years ago

cpethrick commented 2 years ago

Restructuring explicit_ode_solver to use a Butcher tableau for computing Runge-Kutta time-stepping rather than hard-coded. Added a test to check the order of time-stepping.

cpethrick commented 2 years ago

I have made changes according to all comments on formatting, naming etc. I will continue to work on the more labour-intensive suggestions (i.e., moving the grid generation to a more general location; reformulating the current test case to use flow_solver; moving initial conditions to the more general location; and creating an exact solution in the same way as initial conditions).

cpethrick commented 2 years ago

I have made changes according to all comments on formatting, naming etc. I will continue to work on the more labour-intensive suggestions in a future PR (i.e., moving the grid generation to a more general location; reformulating the current test case to use flow_solver; moving initial conditions to the more general location; and creating an exact solution in the same way as initial conditions). Once this PR is approved, I will open an issue for these changes and assign it to myself.

jbrillon commented 2 years ago

Sounds good, can you confirm that all the tests that are expected to pass are indeed passing?

dougshidong commented 2 years ago

Please go ahead and open up the issues you'd assign to yourself. That way the reviewers know that their comments will be taken care by specific issues in future PRs.

Also, let us know if the tests pass on your end.

cpethrick commented 2 years ago

Thanks for the suggestion to run all of the tests. Unfortunately, one of the integration tests now fails (MPI_2D_ADVECTION_EXPLICIT_MANUFACTURED_SOLUTION_LONG) because the explicit pseudotime implementation is not correct. I'm currently trying to debug this, as it should be fixed before this PR is added to the code.

Doug, I will open those issues now.

cpethrick commented 2 years ago

I have fixed the issue which was causing MPI_2D_ADVECTION_EXPLICIT_MANUFACTURED_SOLUTION_LONG to fail. Results now match to the convergence tolerance for explicit pseudotimestepping. Removed an extra call of assemble_residual() which was causing a performance issue, and run-times are similar between master and this branch.

dougshidong commented 2 years ago

Looks good to me.

Thanks again for the great PR and making all those changes.