Feature/cg condition estimate

Support a CG estimate according to Saad. Trivially works in parallel and should be accurate enough for most applications. Performance of CG without condition estimate should not be affected.

Don't know how it could be used for complex numbers, so for now, I just convert the CG coefficients to reals to ensure it at least compiles in that case.

I also fixed the ARPACK wrapper for matrix sizes < 20x20. (Before, it always told ARPACK to construct 20 Arnoldi vectors, which obviously does not work in those cases)

@dominic As for the test, it also passes if no Arpack is found. I guess this is the correct behaviour? (The estimation code also compiles fine without arpack, it only gives a cerr output if Arpack is not present and returns -1 as condition estimate)

added 1 commit

• bef2c07b - Add condition number estimate to CG solver

Compare with previous version

added 1 commit

• 6af39095 - Add condition number estimate to CG solver

Compare with previous version

added 1 commit

• 42f01b11 - Add condition number estimate to CG solver

Compare with previous version

For tests that depend on external libraries to do something meaningful, you can add

dune_add_test(...
              CMAKE_GUARD ARPACK_FOUND
              ...)

It will add a skipped version in case ARpack is not found.

Furthermore, I think adding a reference to a paper that describes the method would be very good style here.

Also, your way of handling std::complex is dangerous IMO: It silently pretends to do something it does not. I would much rather have it tell the user that the feature is not implemented for complex numbers.

I know that dune-istl is in a very messy state wrt our coding style. But it would be nice, if we at least follow it with newly written code, i.e., private members end with an underscore.

added 1 commit

• 9d7b8a00 - Add condition number estimate to CG solver

Compare with previous version

@carsten.graeser I fixed the underscore. I think the rest complies with the coding style.

@dominic I think the test should still be run (although maybe with some warning?) if Arpack is not available, as the estimate still has to compile and fail gracefully in that case. About the complex numbers, is there a way for me to check if the field type is going to work with arpack?

Hmm, it seems like the "gracefulness" of the error right now is totally defined by the user and how good he handles the -1 condition number. It would be good to hear more opinions on that, but I prefer an exception.

added 1 commit

• d4008747 - Add condition number estimate to CG solver

Compare with previous version

added 1 commit

• 447019ec - Add condition number estimate to CG solver

Compare with previous version

Is there a clever way to check for real number types? Of course I could simply restrict support to float and double...

added 1 commit

• 080d2249 - Only support float and double as field types for CG condition estimate

Compare with previous version

Now only float and double are allowed for the condition estimate, any other type leads to a warning and the estimate is disabled.

I would merge this end of next week if nobody objects.

merged

mentioned in commit ab33dd18

mentioned in merge request !120 (merged)

This breaks the CI. One reason for this (std::real is not supported for simd types) is fixed in !120 (merged).

However, the multirhs test still fails for reasons that seem to be located in dune/common/simd.hh.

@carsten.graeser Uuh, I am very sorry about that, I was mislead by the previous passing of CI. @linus.seelinger and me will fix any problems arising after !120 (merged)

The error is fixed with !120 (merged). Please review and merge if this is suitable.

The other issue I mentioned seems to be unrelated and is not revealed by the CI. It already shows up in fmatrixtest in dune-common on Ubuntu 17.04. I open an issue there.

mentioned in issue #39 (closed)

mentioned in issue #40 (closed)