I2PC / scipion

Scipion is an image processing framework to obtain 3D models of macromolecular complexes using Electron Microscopy (3DEM)
http://scipion.i2pc.es
Other
76 stars 47 forks source link

reconstruct_fourier_accel does not compile with gcc7 #1254

Closed azazellochg closed 6 years ago

azazellochg commented 7 years ago

After https://github.com/I2PC/scipion/commit/0884d0989f9804ef50bcb1515d6230439382880b commit, I have the following error during compilation:

software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp: In static member function ‘static Array2D<std::complex<float> >* ProgRecFourierAccel::cropAndShift(MultidimArray<std::complex<double> >&, ProgRecFourierAccel*)’:
software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp:270:57: error: lvalue required as left operand of assignment
     (*result)(j, myPadI).real() = paddedFourierTmp.real();
                                                         ^
software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp:271:57: error: lvalue required as left operand of assignment
     (*result)(j, myPadI).imag() = paddedFourierTmp.imag();
                                                         ^
DStrelak commented 7 years ago

Hello Gregory,

I apologize for the problem. To be honest, I did not check what the problem is. However, there are more commits in the branch that follow this one. It might be that I did not push a commit that is 100% working (which is not good practice, I know). Please refer to a e716ac283b0619d2ca1948779a3b0799150d8913 (last commit on the branch) or to current 'devel' branch, as all changes have been merged already (see 4bf6e9468f26cafeb3cc306b349bf63feb7542f4).

Please let me know if the problem persist.

Kind regards,

David

azazellochg commented 7 years ago

Hello David, no worries, I expect devel to be unstable sometimes :) I have tried to checkout both commits and compile without success. It's always same error.

DStrelak commented 7 years ago

Hello again,

I am not able to reproduce it with GCC 5.4.1. I have run into some issues while trying to update to GCC 7, which would take quite some time to overcome. Can you please try to replace those lines with more explicit form: (result->operator ()(j, myPadI)).real() = paddedFourierTmp.real(); (result->operator ()(j, myPadI)).imag() = paddedFourierTmp.imag();

azazellochg commented 7 years ago

@DStrelak , still the same error :(

software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp: In static member function 'static Array2D<std::complex<float> >* ProgRecFourierAccel::cropAndShift(MultidimArray<std::complex<double> >&, ProgRecFourierAccel*)':
software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp:270:69: error: lvalue required as left operand of assignment
     (result->operator ()(j, myPadI)).real() = paddedFourierTmp.real();
                                                                     ^
software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp:271:69: error: lvalue required as left operand of assignment
     (result->operator ()(j, myPadI)).imag() = paddedFourierTmp.imag();
                                                                     ^
scons: *** [software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.os] Error 1
scons: building terminated because of errors.
DStrelak commented 7 years ago

That is unfortunate. In that case, can you please provide me with full log + you system specification (OS, compiler, scipion config file etc.)? You can either attach it here or upload to https://drive.google.com/drive/folders/0B49nMjU0LV5iSlM1eDFCVjRtZTQ?usp=sharing I will then try to reproduce it at my system. If that fails, would it be possible to create an account for me at your system?

azazellochg commented 7 years ago

OS: Linux xps13-9350 4.12.4-1-ARCH #1 SMP PREEMPT Fri Jul 28 18:54:18 UTC 2017 x86_64 GNU/Linux Config file:

[BUILD]
CC = gcc
CCFLAGS = -std=c99
CUDA = False
CXX = g++
CXXFLAGS = 
DEBUG = True
GTEST = False
JAR = %(JAVA_BINDIR)s/jar
JAVAC = %(JAVA_BINDIR)s/javac
JAVA_BINDIR = %(JAVA_HOME)s/bin
JAVA_HOME = /usr/lib64/jvm/java-8-openjdk
JNI_CPPPATH = %(JAVA_HOME)s/include:%(JAVA_HOME)s/include/linux
LINKERFORPROGRAMS = g++
LINKFLAGS = 
MATLAB = False
MATLAB_DIR = /usr/local/MATLAB/R2011a
MPI_BINDIR = /home/azazello/soft/scipion/software/em/eman-2.2/bin
MPI_CC = mpicc
MPI_CXX = mpiCC
MPI_INCLUDE = /home/azazello/soft/scipion/software/em/eman-2.2/include
MPI_LIB = mpi
MPI_LIBDIR = /home/azazello/soft/scipion/software/em/eman-2.2/lib
MPI_LINKERFORPROGRAMS = mpiCC
NVCC = nvcc --x cu
OPENCV = False
REMOTE_MESA_LIB = /services/guacamole/usr/mesa/lib/
SCIPION_DEBUG_NOCLEAN = True
NVCC_INCLUDE = /usr/local/cuda/include
NVCC_LIBDIR = CUDA_LIB

Verbose output from compiler:

[azazello@xps13-9350 scipion]$ g++ -v -o software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.os -c -g -fPIC -Isoftware/em/xmipp/external/bilib -Isoftware/em/xmipp/external/bilib/headers -Isoftware/em/xmipp/external/bilib/types -Isoftware/em/xmipp/external -Isoftware/em/xmipp/libraries -Isoftware/include software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp
Using built-in specs.
COLLECT_GCC=g++
Target: x86_64-pc-linux-gnu
Configured with: /build/gcc/src/gcc/configure --prefix=/usr --libdir=/usr/lib --libexecdir=/usr/lib --mandir=/usr/share/man --infodir=/usr/share/info --with-bugurl=https://bugs.archlinux.org/ --enable-languages=c,c++,ada,fortran,go,lto,objc,obj-c++ --enable-shared --enable-threads=posix --enable-libmpx --with-system-zlib --with-isl --enable-__cxa_atexit --disable-libunwind-exceptions --enable-clocale=gnu --disable-libstdcxx-pch --disable-libssp --enable-gnu-unique-object --enable-linker-build-id --enable-lto --enable-plugin --enable-install-libiberty --with-linker-hash-style=gnu --enable-gnu-indirect-function --disable-multilib --disable-werror --enable-checking=release --enable-default-pie --enable-default-ssp
Thread model: posix
gcc version 7.1.1 20170630 (GCC) 
COLLECT_GCC_OPTIONS='-v' '-o' 'software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.os' '-c' '-g' '-fPIC' '-I' 'software/em/xmipp/external/bilib' '-I' 'software/em/xmipp/external/bilib/headers' '-I' 'software/em/xmipp/external/bilib/types' '-I' 'software/em/xmipp/external' '-I' 'software/em/xmipp/libraries' '-I' 'software/include' '-shared-libgcc' '-mtune=generic' '-march=x86-64'
 /usr/lib/gcc/x86_64-pc-linux-gnu/7.1.1/cc1plus -quiet -v -I software/em/xmipp/external/bilib -I software/em/xmipp/external/bilib/headers -I software/em/xmipp/external/bilib/types -I software/em/xmipp/external -I software/em/xmipp/libraries -I software/include -D_GNU_SOURCE software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp -quiet -dumpbase reconstruct_fourier_accel.cpp -mtune=generic -march=x86-64 -auxbase-strip software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.os -g -version -fPIC -o /tmp/ccQbsAEK.s
GNU C++14 (GCC) version 7.1.1 20170630 (x86_64-pc-linux-gnu)
        compiled by GNU C version 7.1.1 20170630, GMP version 6.1.2, MPFR version 3.1.5-p2, MPC version 1.0.3, isl version isl-0.18-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
ignoring nonexistent directory "/usr/lib/gcc/x86_64-pc-linux-gnu/7.1.1/../../../../x86_64-pc-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
 software/em/xmipp/external/bilib
 software/em/xmipp/external/bilib/headers
 software/em/xmipp/external/bilib/types
 software/em/xmipp/external
 software/em/xmipp/libraries
 software/include
 /usr/lib/gcc/x86_64-pc-linux-gnu/7.1.1/../../../../include/c++/7.1.1
 /usr/lib/gcc/x86_64-pc-linux-gnu/7.1.1/../../../../include/c++/7.1.1/x86_64-pc-linux-gnu
 /usr/lib/gcc/x86_64-pc-linux-gnu/7.1.1/../../../../include/c++/7.1.1/backward
 /usr/lib/gcc/x86_64-pc-linux-gnu/7.1.1/include
 /usr/local/include
 /usr/lib/gcc/x86_64-pc-linux-gnu/7.1.1/include-fixed
 /usr/include
End of search list.
GNU C++14 (GCC) version 7.1.1 20170630 (x86_64-pc-linux-gnu)
        compiled by GNU C version 7.1.1 20170630, GMP version 6.1.2, MPFR version 3.1.5-p2, MPC version 1.0.3, isl version isl-0.18-GMP

GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 3ffbd954f82c5a86297a549529114e3b
software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp: In static member function ‘static Array2D<std::complex<float> >* ProgRecFourierAccel::cropAndShift(MultidimArray<std::complex<double> >&, ProgRecFourierAccel*)’:
software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp:270:57: error: lvalue required as left operand of assignment
     (*result)(j, myPadI).real() = paddedFourierTmp.real();
                                                         ^
software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp:271:57: error: lvalue required as left operand of assignment
     (*result)(j, myPadI).imag() = paddedFourierTmp.imag();
                                                         ^

Please do ask if you need anything else. PS. I will do system update, will see if new gcc version fixes this. Update: same thing with gcc 7.2.0

DStrelak commented 7 years ago

Hello, talking about gcc... it seems that I (and other people who tried it) have header files that are not according to standard. Please try to replace the problematic lines with: (*result)(j, myPadI) = std::complex(paddedFourierTmp.real(), paddedFourierTmp.imag());

Apparently, my header files defines imag() and real() as T& real(); but standard as T real() const; which would explain the error

azazellochg commented 6 years ago

Hi, ok, I have replaced this, got an error:

software/em/xmipp/libraries/reconstruction/reconstruct_fourier_accel.cpp:270:40: error: missing template arguments before '(' token
     (*result)(j, myPadI) = std::complex(paddedFourierTmp.real(), paddedFourierTmp.imag());
                                        ^

I have replaced the line with (*result)(j, myPadI) = std::complex<double>(paddedFourierTmp.real(), paddedFourierTmp.imag()); and then it worked. Is this a correct solution? (I have no knowledge of c++)

DStrelak commented 6 years ago

Hi, it's really funny how 'forgiving' are some compilers. Never mind, you fixed it almost correctly (should be std::complex, but I guess compiler would take care of it). I will create a patch. Can I set you as a reviewer (reviewer is expected to check that the change is correct and problem is fixed)?

azazellochg commented 6 years ago

@DStrelak sure!