Replace content of MGTransferMatrixFree by that of MGTransferMF

[peterrum]

in reference to #16292 (comment)

[gassmoeller]

Thanks for moving so quickly on this!

I can confirm that this change fixes the issue discussed in #16292. It does however trigger another assert in some of our tests that is related to handing over a ghosted vector into the interpolate_to_mg function (complete error below for reference). I have a crude workaround (simply create a non-ghosted vector, call interpolate_to_mg then copy the result in to the ghosted vector and update ghost entries), and we can modify ASPECT to include this fix. However, this raises the question: Should interpolate_to_mg accept ghosted vectors as destination vectors? If the new version is correct in triggering asserts if it gets a ghosted vector then this will be an incompatible change for some codes.

Also let me know if there is something I can do to help with this PR, happy to do a review if you say this is ready to be looked at.

For later reference: Error message for ASPECT's test skip_refinement:

812: --------------------------------------------------------
812: An error occurred in line <970> of file </home/renegassmoeller/software/dealii/include/deal.II/lac/la_parallel_vector.templates.h> in function
812:     void dealii::LinearAlgebra::distributed::Vector<Number, MemorySpace>::compress_start(unsigned int, dealii::VectorOperation::values) [with Number = double; MemorySpace = dealii::MemorySpace::Host]
812: The violated condition was: 
812:     vector_is_ghosted == false
812: Additional information: 
812:     Cannot call compress() on a ghosted vector
812: 
812: Stacktrace:
812: -----------
812: #0  /home/renegassmoeller/software/dealii/build/lib/libdeal_II.g.so.9.6.0-pre: dealii::LinearAlgebra::distributed::Vector<double, dealii::MemorySpace::Host>::compress_start(unsigned int, dealii::VectorOperation::values)
812: #1  /home/renegassmoeller/software/dealii/build/lib/libdeal_II.g.so.9.6.0-pre: dealii::LinearAlgebra::distributed::Vector<double, dealii::MemorySpace::Host>::compress(dealii::VectorOperation::values)
812: #2  /home/renegassmoeller/software/aspect/build/aspect: void dealii::MGTransferMatrixFree<2, double>::interpolate_to_mg<dealii::LinearAlgebra::distributed::Vector<double, dealii::MemorySpace::Host> >(dealii::DoFHandler<2, 2> const&, dealii::MGLevelObject<dealii::LinearAlgebra::distributed::Vector<double, dealii::MemorySpace::Host> >&, dealii::LinearAlgebra::distributed::Vector<double, dealii::MemorySpace::Host> const&) const
812: #3  /home/renegassmoeller/software/aspect/build/aspect: aspect::MeshDeformation::MeshDeformationHandler<2>::update_multilevel_deformation()
812: #4  /home/renegassmoeller/software/aspect/build/aspect: aspect::MeshDeformation::MeshDeformationHandler<2>::compute_mesh_displacements_gmg()
812: #5  /home/renegassmoeller/software/aspect/build/aspect: aspect::MeshDeformation::MeshDeformationHandler<2>::setup_dofs()
812: #6  /home/renegassmoeller/software/aspect/build/aspect: aspect::Simulator<2>::setup_dofs()
812: #7  /home/renegassmoeller/software/aspect/build/aspect: aspect::Simulator<2>::refine_mesh(unsigned int)
812: #8  /home/renegassmoeller/software/aspect/build/aspect: aspect::Simulator<2>::maybe_do_initial_refinement(unsigned int)
812: #9  /home/renegassmoeller/software/aspect/build/aspect: aspect::Simulator<2>::run()
812: #10  /home/renegassmoeller/software/aspect/build/aspect: void run_simulator<2>(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, bool, bool)
812: #11  /home/renegassmoeller/software/aspect/build/aspect: main

[tjhei]

@peterrum Two comments before we go this way:

1. Have you done any large scale performance tests for setup and transfer? I want to make sure we don't regress.
2. I understand that you want people to migrate to the newer and more correct version but, as can be seen by ASPECT, it is not a 100% replacement. Wouldn't it make more sense just to deprecate the old class?

[peterrum]

For later reference: Error message for ASPECT's test skip_refinement:

@gassmoeller The last commit should fix this. We made a similar change in #15865.

Have you done any large scale performance tests for setup and transfer? I want to make sure we don't regress.

I can run some of the large-scale experiments from the global-coarsening paper.

I understand that you want people to migrate to the newer and more correct version but, as can be seen by ASPECT, it is not a 100% replacement. Wouldn't it make more sense just to deprecate the old class?

It should be a 100% replacement. If this is not the case, there aspects not tested in deal.II.

[gassmoeller]

@peterrum: Thank you! This version can now run all of the ASPECT tests successfully. I cannot check accuracy right now, but at least we didnt trigger any asserts and didnt encounter any crashes.

[peterrum]

As promised, here are some scaling results run with https://github.com/peterrum/dealii-multigrid (annulus, k=1) on SuperMUC-NG. The first plot shows the setup costs and the second plot the computation times. The rows show different number of nodes while the columns show different refinement levels.

One can see that the setup costs are more expensive, while the computation is at least as good as in the old implementation.

[peterrum]

Thank you! This version can now run all of the ASPECT tests successfully. I cannot check accuracy right now, but at least we didnt trigger any asserts and didnt encounter any crashes.

@gassmoeller Thank you for checking!