This was compiled with the latest version of Deal.ii 9.3.0-pre. #61
Conversation
Added: - Operators class - Unit tests verifying all operators: volume, surface, flux, and metric operators and functions Part 1 of my latest merge
|
As discussed, please run all the tests and provide a log of the test to confirm everything still works. |
|
The latest commit reversed back to the forked version of dealii and all files that same as master. The only changes are operators.cpp/h and the unit tests. It passes the previous failing tests too (see attached). Once the header file is properly cleaned up as per Doxygen standards, then it will be ready for pull. |
| prm.declare_entry("use_energy", "false", | ||
| dealii::Patterns::Bool(), | ||
| "Not calculate energy by default. Otherwise, get energy per iteration."); | ||
|
|
||
| prm.declare_entry("use_L2_norm", "false", | ||
| dealii::Patterns::Bool(), | ||
| "Not calculate L2 norm by default (M+K). Otherwise, get L2 norm per iteration."); | ||
|
|
||
| prm.declare_entry("use_classical_Flux_Reconstruction", "false", | ||
| dealii::Patterns::Bool(), | ||
| "Not use Classical Flux Reconstruction by default. Otherwise, use Classical Flux Reconstruction."); | ||
|
|
||
| prm.declare_entry("flux_reconstruction", "cDG", | ||
| dealii::Patterns::Selection("cDG | cSD | cHU | cNegative | cNegative2 | cPlus | cPlus1D |c10Thousand | cHULumped"), | ||
| "Flux Reconstruction. " | ||
| "Choices are <cDG | cSD | cHU | cNegative | cNegative2 | cPlus | cPlus1D | c10Thousand | cHULumped>."); | ||
|
|
||
| prm.declare_entry("flux_reconstruction_aux", "kDG", | ||
| dealii::Patterns::Selection("kDG | kSD | kHU | kNegative | kNegative2 | kPlus | k10Thousand"), | ||
| "Flux Reconstruction for Auxiliary Equation. " | ||
| "Choices are <kDG | kSD | kHU | kNegative | kNegative2 | kPlus | k10Thousand>."); | ||
|
|
There was a problem hiding this comment.
I'm guessing some of those refer to some of your test parameters? Like whether you calculate the energy or l2 norm within the test? You might want to put those in a different subsection. We can merge this pull request an open an issue assigned to you to be fixed later? How does that sound?
There was a problem hiding this comment.
Yes these are all parameters that will be used/properly defined in merge part 2. Basically energy = M+K norm, l2 = M norm, classical flux reconstruction is only reconstructing the surface integral, then the parameters correspond to all the different FR schemes.
Sounds good leaving it as an open issue, it will be fixed in merge part 2!
|
|
||
| prm.declare_entry("use_classical_Flux_Reconstruction", "false", | ||
| dealii::Patterns::Bool(), | ||
| "Not use Classical Flux Reconstruction by default. Otherwise, use Classical Flux Reconstruction."); |
There was a problem hiding this comment.
I don't exactly understand the option.
There was a problem hiding this comment.
This only reconstructs the surface. So for nonlinear problems, its a good way of showing that ESFR with split forms is unstable unless you apply our volume reconstruction to the split form.
| } | ||
| } | ||
| } | ||
| #if 0 |
There was a problem hiding this comment.
Feel free to delete those #if 0 or comment blocks for a cleaner code if you are done debugging. Since you have commited them they can always be retrieved within Git's log by checkout out previous commits. If it's a work in progress, of course, please do keep them.
There was a problem hiding this comment.
Yes, I left those for now because it has all the covariant curl coded up. Was thinking of maybe adding it as an option? Like we could choose between invariant and conservative curl forms from the control file. If you'd like, we could open it as a topic/issue to be addressed in merge part 2?
There was a problem hiding this comment.
Yup! More to-dos to be added to issues
There was a problem hiding this comment.
Sorry I thought this was in a different place. Yes those if statements are needed either way. What this is doing is comparing that M+K_HU recovers exactly a collocated DG mass matrix on GLL nodes of degree p+1. So the if statements are comparing them and writing into M_K_HU the maximum error.
| #include <deal.II/meshworker/dof_info.h> | ||
| #include <deal.II/meshworker/integration_info.h> | ||
| #include <deal.II/meshworker/simple.h> | ||
| #include <deal.II/meshworker/loop.h> |
There was a problem hiding this comment.
Do you need all those headers? Or were they copy pasted? Ideally you only include what you need, which should decrease dependencies and compilation times.
There was a problem hiding this comment.
Honestly I copied and pasted most of them. I will go through them/clean up now.
| { | ||
| pcout << "Destructing Operators..." << std::endl; | ||
| } | ||
| template <int dim, int nstate, typename real> |
There was a problem hiding this comment.
I'm guessing this is copy pasted from the DG constructor. Consider moving that function our of DG, and use the same function for both DG and the Operator constructors. Right now, since this is copy pasted, we would have to change code at 2 locations if we find a bug or want to change something in the framework.
There was a problem hiding this comment.
Yes, this is part of the focus for merge part 2. Since in part 2 I will be changing a fair bit in DG, I figured I will leave it untouched for the first part and just introduce the operators class.
|
All expected tests pass. |
Added: - Operators class
- Unit tests verifying all operators: volume, surface, flux, and metric operators and functions
Part 1 of my latest merge