search

dougshidong / PHiLiP

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

Naca0012 flowsolver #134

Closed donovan97 closed 2 years ago

donovan97 commented 2 years ago

Pull request to add NACA0012 case to FlowSolver.

Main change outside of FlowSolver and related classes is in initial_condition.cpp, to replace nstate variable by a template parameter so that the initial conditions factory is compatible with FreeStreamInitialConditions.

I have also reverted the max refinements to 10, as all the Burger Rewienski tests require this and would currently be failing in the master branch.

dougshidong commented 2 years ago

Added @cpethrick since she is currently getting up to speed to FlowSolver.

Also will be adding @MayaTatarelli after the invitation to collab has been accepted since she might be running more aerodynamic cases this summer.

Even though they might both not be familiar with the current structure, it's a good opportunity to get exposure on his side of the code.

donovan97 commented 2 years ago

Looks great, just a few comments to address. Very useful PR, this is the first aerodynamic case added to the FlowSolver. General comment, since this is now packaged within the FlowSolver class, we could significantly simplify existing tests like src/testing/euler_naca0012.cpp etc by doing something like flow_solver = create_FlowSolver inside then doing flow_solver->run_test(); similar to how its done in src/testing/taylor_green_vortex_energy_check.cpp and in src/testing/taylor_green_vortex_restart_check.cpp with the parameter objects; consider opening an issue for this, listing all the relevant src/testing/ files that can be simplified

Sure, I'll open an issue once we merge to master.

dougshidong commented 2 years ago

Looks good to me. Comments I left are prime examples of issues arising from my past self leaving dead code which causes further confusion.

dougshidong commented 2 years ago

I think we've got enough eyes on this small PR.

Reviewers can resolve their comment when satisfied. @MayaTatarelli , feel free to press "Squash and merge" whenever you're happy with it.