OPM / opm-simulators

OPM Flow and experimental simulators, including components such as well models etc.
http://www.opm-project.org
GNU General Public License v3.0
123 stars 121 forks source link

build error on ubuntu 18.04 #1517

Closed andlaus closed 2 years ago

andlaus commented 6 years ago

when trying to compile opm-simulators on my ubuntu 18.04 laptop, I get this:

> make
[  0%] Building CXX object CMakeFiles/opmsimulators.dir/opm/simulators/flow_ebos_blackoil.cpp.o
In file included from /usr/include/dune/istl/superlu.hh:22:0,
                 from /usr/include/dune/istl/paamg/amg.hh:13,
                 from /home/and/src/opm-simulators/opm/autodiff/CPRPreconditioner.hpp:41,
                 from /home/and/src/opm-simulators/opm/autodiff/BlackoilAmg.hpp:23,
                 from /home/and/src/opm-simulators/opm/autodiff/ISTLSolver.hpp:23,
                 from /home/and/src/opm-simulators/opm/autodiff/StandardWell.hpp:28,
                 from /home/and/src/opm-simulators/opm/autodiff/BlackoilWellModel.hpp:50,
                 from /home/and/src/opm-simulators/opm/autodiff/BlackoilModelEbos.hpp:31,
                 from /home/and/src/opm-simulators/opm/autodiff/SimulatorFullyImplicitBlackoilEbos.hpp:27,
                 from /home/and/src/opm-simulators/opm/simulators/flow_ebos_blackoil.cpp:26:
/usr/include/superlu/slu_ddefs.h:267:12: error: conflicting declaration of C function ‘int dgemm_(const char*, const char*, const int*, const int*, const int*, const double*, const double*, const int*, const double*, const int*, const double*, double*, const int*)’
 extern int dgemm_(const char*, const char*, const int*, const int*, const int*,
            ^~~~~~
In file included from /home/and/src/opm-grid/opm/grid/transmissibility/TransTpfa_impl.hpp:1:0,
                 from /home/and/src/opm-grid/opm/grid/transmissibility/TransTpfa.hpp:104,
                 from /home/and/src/opm-simulators/opm/autodiff/GeoProps.hpp:26,
                 from /home/and/src/opm-simulators/opm/autodiff/AutoDiffHelpers.hpp:26,
                 from /home/and/src/opm-simulators/opm/autodiff/VFPHelpers.hpp:27,
                 from /home/and/src/opm-simulators/opm/autodiff/VFPInjProperties.hpp:27,
                 from /home/and/src/opm-simulators/opm/autodiff/WellInterface.hpp:37,
                 from /home/and/src/opm-simulators/opm/autodiff/BlackoilWellModel.hpp:49,
                 from /home/and/src/opm-simulators/opm/autodiff/BlackoilModelEbos.hpp:31,
                 from /home/and/src/opm-simulators/opm/autodiff/SimulatorFullyImplicitBlackoilEbos.hpp:27,
                 from /home/and/src/opm-simulators/opm/simulators/flow_ebos_blackoil.cpp:26:
/home/and/src/opm-common/opm/common/utility/numeric/blas_lapack.h:39:6: note: previous declaration ‘void dgemm_(const char*, const char*, const int*, const int*, const int*, const double*, const double*, const int*, const double*, const int*, const double*, double*, const int*)’
 void dgemm_(const char *transA  , const char *transB   ,
      ^~~~~~

this is due to conflicting forward declarations of the BLAS function dgemm_ by superlu and opm-common's opm-common/opm/common/utility/numeric/blas_lapack.h. since as far as I'm aware, BLAS is not used by non-legacy flow, I propose to remove the functionality or if the fallout turns out to be too much work, make it possible to compile flow without having to compile any legacy code.

Also note that it is quite easy to trigger this error: starting on a vanilla ubuntu 18.04, the following has been done to install the prerequisites:

apt-add-repository ppa:opm/ppa
apt update
apt install g++ cmake git libtrilinos-zoltan-dev libeigen3-dev libboost-all-dev libdune-*-dev
atgeirr commented 6 years ago
  1. The declarations look identical to me from a type perspective, are we not allowed to declare the same function multiple times in C?
  2. Did we not get rid of SuperLU? Was it just a dream...?
  3. A very simple way to get rid of this problem is to move the implementation currently in TransTpfa_impl.hpp into a cpp file, and explicitly instantiate the template for the 2 grid types. Although it might reappear in a different context, it should be a good workaround. What do you think?
bska commented 6 years ago

The declarations look identical to me from a type perspective, are we not allowed to declare the same function multiple times in C?

Not with different return types.

akva2 commented 6 years ago

1) differeing return types 2) we disabled it by default, but it can still be enabled. superlu in bionic is broken, it is prototyping blas functions wrongly. blas never had return values. 3) yes some include level workaround should probably be employed.

atgeirr commented 6 years ago

Not with different return types.

Ah, thanks! According to this (somewhat randomly googled) page:

http://www.netlib.org/clapack/cblas/dgemm.c

... it looks like perhaps we should go for int rather than void in blas_lapack.h as well => problem solved?

atgeirr commented 6 years ago

blas never had return values.

Not in Fortran, but from C it is less clear to me how it should be done, is the link I posted also using the wrong prototype?

bska commented 6 years ago

looks like perhaps we should go for int rather than void in blas_lapack.h as well

We're not allowed to. BLAS subroutines don't have return values in the traditional sense. Declaring an int return type means we assume that the underlying implementation is CBLAS rather than reference BLAS or, say, GotoBLAS.

akva2 commented 6 years ago

there are two ways to do blas from C, either by importing the fortran interface, or by using the f2c'd style interface, which gives you 'cblas', wrapping stuff and having return values. this is afaict the blas iface, so it's messy.

andlaus commented 6 years ago

is there a problem with just not using BLAS anymore and remove that header? as far as I can see, the stuff that flow_legacy uses can be quite easily be replaced by the dense linear algebra classes of dune-common.

andlaus commented 6 years ago

... can be quite easily be replaced by the dense linear algebra classes of dune-common.

BTW: I'm pretty sure that this would even come with a small speedup if the objects in question are small.

andlaus commented 6 years ago

I just checked: the only places where any BLAS functionality is used are TransTpfa_impl.hpp, some (probably) long unused code that uses the old fluid properties interface and some old C stuff that was inherited from opm-core. the C stuff is probably unused even for flow_legacy and TransTpfa only uses dgemm_()

blattms commented 2 years ago

Closing as I doubt that it still applies (packages build correctly at least).