dealii / dealii

The development repository for the deal.II finite element library
https://www.dealii.org
Other
1.34k stars 741 forks source link

Problems with Intel compiler and complex long double #2551

Closed kronbichler closed 8 years ago

kronbichler commented 8 years ago

When trying to compile the library with the Intel compiler, version 16, I get the (many) of the following erros:

/home/kronbichler/sw/deal.II/include/deal.II/lac/constraint_matrix.templates.h(1604): error: no operator "*" matches these operands
            operand types are: const std::complex<long double> * double
            col_val += (local_matrix(loc_row, global_cols.local_row(j,p)) *
                                                                          ^
[...]
            instantiation of "void dealii::ConstraintMatrix::distribute_local_to_global(const dealii::FullMatrix<MatrixType::value_type> &, const dealii::Vector<VectorType::value_type> &,
            const std::vector<unsigned int, std::allocator<unsigned int>> &, MatrixType &, VectorType &, bool, dealii::internal::bool2type<false>) const
            [with MatrixType=dealii::SparseMatrix<std::complex<long double>>, VectorType=dealii::Vector<std::complex<long double>>]" at line 1305 of
                      "/home/kronbichler/sw/deal.II/source/lac/constraint_matrix.cc"

The problem is that the Intel compiler does not seem to understand multiplication between std::complex<long double> and double. Has anyone else seen this? My solution would be to get rid of std::complex<long double> instantiations, both in special files and as a complex scalar. I am not really sure in which context they might be of use anyway. The trend seems to go away from long double (if there ever was) because the 80 bit long double is getting more of a niche as it is the only format left on the old x87 format.

In case my system is weird, here are my compiler details:

icpc version 16.0.1 (gcc version 4.8.5 compatibility)

(I tried to attach gcc 5 to the Intel compiler but I get many C++11-related errors, it looks like Intel is not ready for that yet.)

tjhei commented 8 years ago

I see the same with

icpc version 16.0.2 (gcc version 4.8.0 compatibility)

The problem seems to be the combination of complex<long double> and double. Casting the second argument to a long double should work.

kronbichler commented 8 years ago

So how should we proceed here? There are several places where this problem appears. The difficult thing is that we would need to change the operator * to work around the issue. I don't particularly like that. In my opinion the three viable options are:

  1. Disable complex long doubles in cmake/config/template-arguments.in unconditionally
  2. Disable complex long doubles in case we detect the compiler does not support mixing arithmetics
  3. Exclude Intel compiler 16

Even though I am not a big fan of the Intel compiler (because it generates various optimization bugs here and there and on top of that is consistently 5-10% slower code on my algorithms), I consider the last option a bit harsh. My favorite would be 1.

I personally would also disable real long double from the default instantiations. I have yet to see a situation where long double on vectors really pays off (apart from well-isolated use e.g. in evaluation of polynomials or Gauss point locations where it helps us to get the last digit of double right). I have not seen any of the other large libraries (Trilinos, PETSc, FEniCS) making as wide use of it as we are. An additional problem is that we have most things hard-wired in double which makes long double questionable anyway. I've done a test comparing float and double evaluation a few years ago and if I remember correctly it turned out that as soon as the polynomial evaluation (FEValues type scheme) was done in float, you are essentially down to float precision everywhere even if you have vectors and matrices in double format. Finally, if one needs additional accuracy, I would go for float128 (quad precision). If one also needs performance, software-managed double-double precision is probably not inferior to long double and considerably more accurate.

davydden commented 8 years ago

I think (1) is probably the best way.

It is certainly an option to remove real long double from the instantiations as well.

tjhei commented 8 years ago

I personally would also disable real long double from the default instantiations

Has anyone used long doubles successfully or is there a specific use case I am missing? Otherwise I would get rid of it.

kronbichler commented 8 years ago

Unless there are dozens of people using long double, I think the appropriate thing would be that those few people who use it get the necessary code via including the .templates.h files. If that mechanism does not work, we should rather improve the library than always enable long double.

tjhei commented 8 years ago

There doesn't seem to be a single test for SparseMatrix<std::complex<long double>> so I would consider this combination broken anyways. ;-)

Rombur commented 8 years ago

I don't know anyone using long double either. I think we can get rid of it.

bangerth commented 8 years ago

Let's get rid of the long double instantiations. It's one of those cases from 15 years ago where I did something because it could be done, not because there was a need. I regret these choices every time I come by it today and would not do them again.

kronbichler commented 8 years ago

Alright, I will try to prepare a patch. (So there is still some time for those who have not had the time to react by now.) I just made a short search and the number of appearances of long double is not dramatic. Taking away the places in the polynomials and quadrature where they have their role it's probably not more than 50 which seems to be doable in an hour or two.