Closed donovan97 closed 2 years ago
I added @cpethrick since she's working with the ODE solver
Great PR. The addition of the Eigen library seems very convenient, and it looks like you've put significant effort towards improving the reduced-order capabilities of PHiLIP. There's already been some great discussion already about the ODE Solver section, which I contributed to where I saw fit. I have a few fairly minor comments.
Does the addition of the Eigen library necessitate an update to the documentation/readme/installation files? If so, that should be added to your PR.
Since Eigen was added as a submodule, git should handle it automatically for a new installation, like the other existing submodules. For anyone who already has PHiLiP installed, you will likely need to update your git submodules using the command:
git submodule update --init --recursive
Thanks everyone for the thorough review and great job. There was a relatively high number of changes and the review comments were all insightful. Thanks for making the changes as well Donovan.
Keep it up.
Pull request for goal-oriented adaptive sampling for reduced-order models and the addition of the Eigen library to the code.
The pull request introduces significant changes to the reduced-order modeling. All of the old reduced-order modeling code is replaced as it did not work for cases run on multiple processors. Unused code and tests were also removed.
Summary of main changes:
Changes to
Functional
class:dealii::TrilinosWrappers::SparseMatrix
are changed to pointers. This is because thedealii::TrilinosWrappers::SparseMatrix
class has avirtual
destructor to use pointers to the class, see https://www.dealii.org/current/doxygen/deal.II/classTrilinosWrappers_1_1SparseMatrix.html#adbe89a97bc7ef4bbc16e5e04b72bb19e As a result of thevirtual
destructor, I was encountering errors due to thedealii::TrilinosWrappers::SparseMatrix
within theFunctional
class when theFunctional
object in thefunctionalFactory
method (pod_adaptive_sampling.cpp
) went out of scope. The solution was to make theSparseMatrices
pointers, and use a pointer to the Functional base class for thefunctionalFactory
function inpod_adaptive_sampling.cpp
. The destructor for theFunctional
class is also made virtual as this is supposed to be the case for a base class. Update July 11: ThefunctionalFactory
method inpod_adaptive_sampling.cpp
was removed and instead theFunctionalFactory
class infunctional.cpp
is being used.ODE solver changes:
Test changes:
reduced_order.cpp
now tests for consistency of the reduced-order model. Tested on Burgers Rewienski and NACA0012 FlowSolver cases.pod_adaptive_sampling.cpp
performs an adaptive snapshot sampling for the reduced-order model. Tested on Burgers Rewienski and NACA0012.adaptive_sampling_testing.cpp
is used for validation testing and producing additional results, it does not actually test anything. Tests using this will be commented out before merging.reduced_order directory changes:
pod_interface.h
provides an interface for the types of POD (offline and online POD)pod_basis_offline.cpp
computes a POD basis from previously computed snapshotspod_basis_online.cpp
computes a POD basis "online", during the adaptive sampling processmin_max_scaler.cpp
scales data between 0 and 1, used before interpolatingrbf_interpolation.cpp
implements a radial basis function interpolationnearest_neighbors.cpp
implements k nearest neighbors for use with adaptive samplingreduced_order_solution.cpp
stores information about a reduced order solution that is required in future iterations of adaptive samplingrom_test_location.cpp
computes adjoint-based error estimates for adaptive samplinghalton.cpp
computes a Halton sequence. I did not write this file, no need to review it.Addition of Eigen library:
The pull request also adds the Eigen library as a submodule (see .gitmodules). Therefore, after merging the PR, everyone will have to update their git submodules to download the Eigen library in the submodules directory. This may need to be done with the command:
git submodule update --init --recursive
All required tests are passing and the addition of Eigen is not causing issues on Narval.