search

dougshidong / PHiLiP

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

Better non-physical value handling and adding parmeters object to Physics #214

Closed cpethrick closed 1 year ago

cpethrick commented 1 year ago

A recent change prints warning messages whenever a negative density/pressure is detected in Euler. This is helpful for some tests, but results in very long log files for others (for instance, where an iterative solver sometimes fails but the test is built to handle this). Therefore, it is useful to add a parameter which will give the user more control over behaviour in these cases.

A function has been added to PhysicsBase such that it could be used to handle non-physical values in any PDE. It is only called in Euler, but other PDEs should be modified to use it in the future.

As an impact of this change, many constructors needed to be changed. Rather than adding a single new parameter, I have added a parameters pointer to PhysicsBase. I think this is a good structural addition. Constructors in physics_factory and model_factory have gotten very complex, and having a parameters object could allow us to simplify these constructors in the future.

Note that a dummy parameters pointer had to be added to all physics unit tests to be compatible with the changes to constructors.

CTEST passes as before.

cpethrick commented 1 year ago

Marking this as ready for review.

After I have received and responded to feedback, I intend to open two issues for future work:

  1. Simplify constructors in physics to take only the params object and read relevant data from params, rather than passing select values in the factory. Possibly overload the constructor for compatibility with old tests (especially the unit tests which were modified in this PR).
  2. Add error-handling to other physics to catch NaN or non-physical negative values.

Both should be fairly straightforward changes, but are out of the scope of this PR.

cpethrick commented 1 year ago

I ran ctest on 6 processes locally and most tests are passing. The results are attached. Please note that this was run one commit behind master, so the following testfails have been fixed by PR #224: 174 - 1D_BURGERS_STABILITY_ENERGY_LONG (Failed) 177 - MPI_3D_EULER_SPLIT_TAYLOR_GREEN (Failed) 178 - MPI_3D_EULER_SPLIT_TAYLOR_GREEN_CURV (Failed) 182 - MPI_2D_ADVECTION_EXPLICIT_PERIODIC_ENERGY_WEIGHT_ADJUSTED_ON_THE_FLY_LONG (Failed) I've ran each of these individually after merging to the current master and they pass as expected.

ctestlog.txt