Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lanczos tests #132

Closed
wants to merge 3 commits into from
Closed

Lanczos tests #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c84a945
Add MatrixMarket output for dealii::SparseMatrix
aprokop Feb 18, 2019
767ae8e
Trying to make Lanczos more robust
aprokop Feb 18, 2019
8b1fa40
Adding Lanczos integration testing
aprokop Feb 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion include/mfmg/dealii/amge_host.templates.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ AMGe_host<dim, MeshEvaluator, VectorType>::compute_local_eigenvectors(
{
boost::property_tree::ptree lanczos_params;
lanczos_params.put("num_eigenpairs", n_eigenvectors);
lanczos_params.put("tolerance", tolerance);
// lanczos_params.put("tolerance", tolerance);
lanczos_params.put("tolerance", 1e-6);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to commit that?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but this is all in progress. I need to put a lot more comments about why I'm doing things this way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do so.

lanczos_params.put("max_iterations",
_eigensolver_params.get("max_iterations", 200));
lanczos_params.put("percent_overshoot",
Expand Down
3 changes: 3 additions & 0 deletions include/mfmg/dealii/dealii_utils.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ void matrix_market_output_file(
const std::string &filename,
const dealii::TrilinosWrappers::SparseMatrix &matrix);

void matrix_market_output_file(const std::string &filename,
const dealii::SparseMatrix<double> &matrix);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment on how this gets called with the changes you propose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not, should have been a separate PR for that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove from that PR.


void matrix_market_output_file(
const std::string &filename,
const dealii::TrilinosWrappers::MPI::Vector &vector);
Expand Down
5 changes: 5 additions & 0 deletions include/mfmg/dealii/lanczos.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ class Lanczos
const int num_requested, double tol,
std::vector<double> const &evecs);

static bool
details_check_convergence_stupid(std::vector<double> const &old_evals,
std::vector<double> const &new_evals,
double tolerance);

static std::vector<VectorType>
details_calc_evecs(const int num_requested, const int n,
std::vector<VectorType> const &lanc_vectors,
Expand Down
29 changes: 27 additions & 2 deletions include/mfmg/dealii/lanczos.templates.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,19 @@ Lanczos<OperatorType, VectorType>::details_solve_lanczos(

// Calculate eigenpairs of tridiagonal matrix for convergence test or at
// last iteration
std::tie(evals, evecs_tridiag) =
std::vector<double> iteration_evals;
std::tie(iteration_evals, evecs_tridiag) =
details_calc_tridiag_epairs(t_maindiag, t_offdiag, num_requested);

// Stupid heuristic check
// Cause the regular may do "interesting" things when run long enough
if (evals.size() == num_requested &&
details_check_convergence_stupid(evals, iteration_evals, tol))
{
break;
}
evals = iteration_evals;

if (details_check_convergence(beta, dim_hessenberg, num_requested, tol,
evecs_tridiag))
{
Expand All @@ -241,7 +251,7 @@ Lanczos<OperatorType, VectorType>::details_solve_lanczos(
evecs = details_calc_evecs(num_requested, it, lanc_vectors, evecs_tridiag);

return std::make_tuple(evals, evecs);
}
} // namespace mfmg

/// \brief Lanczos solver: calculate eigenpairs from tridiagonal of Lanczos
/// coefficients
Expand Down Expand Up @@ -323,6 +333,21 @@ bool Lanczos<OperatorType, VectorType>::details_check_convergence(
return is_converged;
}

/// \brief Lanczos solver: perform convergence check (stupid version)
template <typename OperatorType, typename VectorType>
bool Lanczos<OperatorType, VectorType>::details_check_convergence_stupid(
std::vector<double> const &old_evals, std::vector<double> const &new_evals,
double tolerance)
{
ASSERT(old_evals.size() == new_evals.size(), "Size mismatch");

bool is_converged = false;
for (int i = 0; i < old_evals.size(); i++)
is_converged |= (std::abs(old_evals[i] / new_evals[i] - 1.0) < tolerance);

return is_converged;
}

/// \brief Lanczos solver: calculate full (approx) eigenvectors from tridiag
/// eigenvectors
template <typename OperatorType, typename VectorType>
Expand Down
9 changes: 9 additions & 0 deletions source/dealii/dealii_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,15 @@ void matrix_market_output_file(
std::to_string(rv));
}

void matrix_market_output_file(const std::string &filename,
const dealii::SparseMatrix<double> &matrix)
{
dealii::TrilinosWrappers::SparseMatrix trilinos_matrix;
trilinos_matrix.reinit(matrix);

matrix_market_output_file(filename, trilinos_matrix);
}

// TODO: write the map
void matrix_market_output_file(
const std::string &filename,
Expand Down
88 changes: 53 additions & 35 deletions tests/test_hierarchy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ double test(std::shared_ptr<boost::property_tree::ptree> params)
laplace._dof_handler, laplace._constraints, a, material_property);
mfmg::Hierarchy<DVector> hierarchy(comm, evaluator, params);

pcout << "Grid complexity : " << hierarchy.grid_complexity() << std::endl;
pcout << "Operator complexity: " << hierarchy.operator_complexity()
<< std::endl;
// pcout << "Grid complexity : " << hierarchy.grid_complexity() <<
// std::endl; pcout << "Operator complexity: " <<
// hierarchy.operator_complexity()
// << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove do not comment it out.

What is the rational for having/not having these print to the standard output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove them.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check with @Rombur. I don't have an opinion on this and I am not sure why it was here in the 1st place.


// We want to do 20 V-cycle iterations. The rhs of is zero.
// Use D(istributed)Vector because deal has its own Vector class
Expand Down Expand Up @@ -161,8 +162,9 @@ BOOST_DATA_TEST_CASE(
bdata::make({"hyper_cube", "hyper_ball"}) * bdata::make({false, true}) *
bdata::make({"None", "Reverse Cuthill_McKee"}) *
bdata::make<std::string>({"DealIIMeshEvaluator",
"DealIIMatrixFreeMeshEvaluator"}),
mesh, distort_random, reordering, mesh_evaluator_type)
"DealIIMatrixFreeMeshEvaluator"}) *
bdata::make<std::string>({"lapack", "lanczos"}),
mesh, distort_random, reordering, mesh_evaluator_type, eigensolver)
{
// TODO investigate why there is large difference in convergence rate when
// running in parallel.
Expand All @@ -180,7 +182,7 @@ BOOST_DATA_TEST_CASE(
params->put("smoother.type", "Chebyshev");
}

params->put("eigensolver.type", "lapack");
params->put("eigensolver.type", eigensolver);
params->put("agglomeration.nz", 2);
params->put("laplace.n_refinements", 2);
params->put("laplace.mesh", mesh);
Expand All @@ -193,35 +195,51 @@ BOOST_DATA_TEST_CASE(

// This is a gold standard test. Not the greatest but it makes sure we don't
// break the code
std::map<std::tuple<std::string, bool, std::string>, double> ref_solution;
ref_solution[std::make_tuple("hyper_cube", false, "None")] =
is_matrix_free ? 0.1482630509 : 0.0491724046;
ref_solution[std::make_tuple("hyper_cube", false,
"Reverse Cuthill_McKee")] =
is_matrix_free ? 0.1482630509 : 0.0491724046;
ref_solution[std::make_tuple("hyper_cube", true, "None")] =
is_matrix_free ? 0.1575262224 : 0.0488984875;
ref_solution[std::make_tuple("hyper_cube", true, "Reverse Cuthill_McKee")] =
is_matrix_free ? 0.1575262224 : 0.0488984875;
ref_solution[std::make_tuple("hyper_ball", false, "None")] =
ref_solution[std::make_tuple("hyper_ball", false,
"Reverse Cuthill_McKee")] =
is_matrix_free ? 0.3022004744 : 0.1146629782;
ref_solution[std::make_tuple("hyper_ball", true, "None")] =
is_matrix_free ? 0.2977841482 : 0.1024334788;
ref_solution[std::make_tuple("hyper_ball", true, "Reverse Cuthill_McKee")] =
is_matrix_free ? 0.2977841482 : 0.1024334788;

if (mesh == std::string("hyper_cube"))
BOOST_TEST(
conv_rate ==
ref_solution[std::make_tuple(mesh, distort_random, reordering)],
tt::tolerance(1e-6));
else
BOOST_TEST(
conv_rate ==
ref_solution[std::make_tuple(mesh, distort_random, reordering)],
tt::tolerance(1e-6));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment here? Did we have if (mesh is hyper cube) { expression; } else { same expression; }?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

// (geometry, distortion, reordering, eigensolver, matrix-free)
std::map<std::tuple<std::string, bool, std::string, std::string, bool>,
double>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability: I would prefer a string or a scoped enumeration to denote matrix-free mode.

ref_solution;
// clang-format off
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Readability: Good call !

ref_solution[std::make_tuple("hyper_cube" , false , "None" , "lapack" , false)] = 0.0491724046;
ref_solution[std::make_tuple("hyper_cube" , false , "None" , "lapack" , true)] = 0.1482630509;
ref_solution[std::make_tuple("hyper_cube" , false , "Reverse Cuthill_McKee" , "lapack" , false)] = 0.0491724046;
ref_solution[std::make_tuple("hyper_cube" , false , "Reverse Cuthill_McKee" , "lapack" , true)] = 0.1482630509;
ref_solution[std::make_tuple("hyper_cube" , true , "None" , "lapack" , false)] = 0.0488984875;
ref_solution[std::make_tuple("hyper_cube" , true , "None" , "lapack" , true)] = 0.1575262224;
ref_solution[std::make_tuple("hyper_cube" , true , "Reverse Cuthill_McKee" , "lapack" , false)] = 0.0488984875;
ref_solution[std::make_tuple("hyper_cube" , true , "Reverse Cuthill_McKee" , "lapack" , true)] = 0.1575262224;
ref_solution[std::make_tuple("hyper_ball" , false , "None" , "lapack" , false)] = 0.1146629782;
ref_solution[std::make_tuple("hyper_ball" , false , "None" , "lapack" , true)] = 0.3022004744;
ref_solution[std::make_tuple("hyper_ball" , false , "Reverse Cuthill_McKee" , "lapack" , false)] = 0.1146629782;
ref_solution[std::make_tuple("hyper_ball" , false , "Reverse Cuthill_McKee" , "lapack" , true)] = 0.3022004744;
ref_solution[std::make_tuple("hyper_ball" , true , "None" , "lapack" , false)] = 0.1024334788;
ref_solution[std::make_tuple("hyper_ball" , true , "None" , "lapack" , true)] = 0.2977841482;
ref_solution[std::make_tuple("hyper_ball" , true , "Reverse Cuthill_McKee" , "lapack" , false)] = 0.1024334788;
ref_solution[std::make_tuple("hyper_ball" , true , "Reverse Cuthill_McKee" , "lapack" , true)] = 0.2977841482;

ref_solution[std::make_tuple("hyper_cube" , false , "None" , "lanczos" , false)] = 0.0491724046;
ref_solution[std::make_tuple("hyper_cube" , false , "None" , "lanczos" , true)] = 0.1482630509;
ref_solution[std::make_tuple("hyper_cube" , false , "Reverse Cuthill_McKee" , "lanczos" , false)] = 0.0491724046;
ref_solution[std::make_tuple("hyper_cube" , false , "Reverse Cuthill_McKee" , "lanczos" , true)] = 0.1482630509;
ref_solution[std::make_tuple("hyper_cube" , true , "None" , "lanczos" , false)] = 0.0488968016;
ref_solution[std::make_tuple("hyper_cube" , true , "None" , "lanczos" , true)] = 0.1575200977;
ref_solution[std::make_tuple("hyper_cube" , true , "Reverse Cuthill_McKee" , "lanczos" , false)] = 0.0488968016;
ref_solution[std::make_tuple("hyper_cube" , true , "Reverse Cuthill_McKee" , "lanczos" , true)] = 0.1575200977;
ref_solution[std::make_tuple("hyper_ball" , false , "None" , "lanczos" , false)] = 0.0953745739;
ref_solution[std::make_tuple("hyper_ball" , false , "None" , "lanczos" , true)] = 0.3017812295;
ref_solution[std::make_tuple("hyper_ball" , false , "Reverse Cuthill_McKee" , "lanczos" , false)] = 0.0953745739;
ref_solution[std::make_tuple("hyper_ball" , false , "Reverse Cuthill_McKee" , "lanczos" , true)] = 0.3017812295;
ref_solution[std::make_tuple("hyper_ball" , true , "None" , "lanczos" , false)] = 0.0966058675;
ref_solution[std::make_tuple("hyper_ball" , true , "None" , "lanczos" , true)] = 0.2982453175;
ref_solution[std::make_tuple("hyper_ball" , true , "Reverse Cuthill_McKee" , "lanczos" , false)] = 0.0966058675;
ref_solution[std::make_tuple("hyper_ball" , true , "Reverse Cuthill_McKee" , "lanczos" , true)] = 0.2982453175;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR
Would you confirm that you did add convergences rates for Lanczos and that nothing else change? Can you comment on the new gold values? Are they reasonably close to their LAPACK's counterpart?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The values are really close but not sure how robust the thing is. Lanczos test now fails, so I may need to play around with things. The whole thing is annoying, but proper fix is unclear.

// clang-format on

BOOST_TEST(
conv_rate ==
ref_solution[std::make_tuple(mesh, distort_random, reordering,
eigensolver, is_matrix_free)],
tt::tolerance(1e-6));
}
}

Expand Down
12 changes: 9 additions & 3 deletions tests/test_restriction_matrix.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,16 @@
#include <deal.II/lac/la_parallel_vector.h>

#include <boost/property_tree/info_parser.hpp>
#include <boost/test/data/test_case.hpp>

#include <random>

#include "laplace.hpp"
#include "main.cc"

namespace utf = boost::unit_test;
namespace bdata = boost::unit_test::data;
namespace tt = boost::test_tools;

template <int dim>
class DummyMeshEvaluator : public mfmg::DealIIMeshEvaluator<dim>
Expand Down Expand Up @@ -279,7 +282,8 @@ class TestMeshEvaluator : public mfmg::DealIIMeshEvaluator<dim>

// FIXME relaxed tolerance from 1e-14 to 1e-4 for this test to pass while using
// ARPACK's regular mode instead of shift-and-invert
BOOST_AUTO_TEST_CASE(weight_sum, *utf::tolerance(1e-4))
BOOST_DATA_TEST_CASE(weight_sum, bdata::make({"lapack", "lanczos"}),
eigensolver)
{
// Check that the weight sum is equal to one
unsigned int constexpr dim = 2;
Expand All @@ -295,6 +299,7 @@ BOOST_AUTO_TEST_CASE(weight_sum, *utf::tolerance(1e-4))

auto params = std::make_shared<boost::property_tree::ptree>();
boost::property_tree::info_parser::read_info("hierarchy_input.info", *params);
params->put("eigensolver.type", eigensolver);
params->put("eigensolver.number of eigenvectors", 1);
auto agglomerate_ptree = params->get_child("agglomeration");
int n_eigenvectors =
Expand All @@ -311,7 +316,8 @@ BOOST_AUTO_TEST_CASE(weight_sum, *utf::tolerance(1e-4))

TestMeshEvaluator<dim> evaluator(laplace._dof_handler, laplace._constraints,
laplace._system_matrix);
mfmg::AMGe_host<dim, MeshEvaluator, DVector> amge(comm, laplace._dof_handler);
mfmg::AMGe_host<dim, MeshEvaluator, DVector> amge(
comm, laplace._dof_handler, params->get_child("eigensolver"));

auto locally_relevant_global_diag = evaluator.get_diagonal();

Expand All @@ -334,6 +340,6 @@ BOOST_AUTO_TEST_CASE(weight_sum, *utf::tolerance(1e-4))
e[i] = 1.;
e.compress(::dealii::VectorOperation::insert);
restrictor_matrix.vmult(ee, e);
BOOST_TEST(ee.l1_norm() == 1.);
BOOST_TEST(ee.l1_norm() == 1., tt::tolerance(1e-4));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the rational for having the floating point comparison manipulation in the BOOST_TEST macro versus having a decorator for the unit test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not sure that BOOST_AUTO_TEST_CASE and BOOST_DATA_TEST_CASE behave the same way, and was too lazy to google. So just replaced with what I know.

}
}