pelahi / VELOCIraptor-STF

Galaxy/(sub)Halo finder for N-body simulations
MIT License
20 stars 26 forks source link

mass not declared in scope of NBodylib/src/NBody/Particle.cxx #19

Closed broukema closed 6 years ago

broukema commented 6 years ago

PROBLEM:

The variable "mass" on line 226 of NBodylib/src/NBody/Particle.cxx is not declared (not in that scope, anyway), giving a compile error with gcc-6.3.0 .

SYSTEM:

Debian GNU/Linux 4.9.0

dpkg -l |egrep "gcc|autoconf|libtool"

ii autoconf 2.69-10 all automatic configure script builder ii autoconf-archive 20160916-1 all Autoconf Macro Archive ii autoconf-doc 2.69-10 all automatic configure script builder documentation ii gcc 4:6.3.0-4 amd64 GNU C compiler ii gcc-6 6.3.0-18+deb9u1 amd64 GNU C compiler ii gcc-6-base:amd64 6.3.0-18+deb9u1 amd64 GCC, the GNU Compiler Collection (base package) ii gcc-6-doc 6.3.0-1 all documentation for the GNU compilers (gcc, gobjc, g++) ii gcc-doc 5:6.1.0-1 amd64 documentation for the GNU compilers (gcc, gobjc, g++) ii gcc-doc-base 6.1.0-1 all several GNU manual pages ii libgcc-6-dev:amd64 6.3.0-18+deb9u1 amd64 GCC support library (development files) ii libgcc1:amd64 1:6.3.0-18+deb9u1 amd64 GCC support library ii libltdl-dev:amd64 2.4.6-2 amd64 System independent dlopen wrapper for GNU libtool ii libltdl7:amd64 2.4.6-2 amd64 System independent dlopen wrapper for GNU libtool ii libtool 2.4.6-2 all Generic library support script ii linux-compiler-gcc-6-x86 4.9.110-3+deb9u4 amd64 Compiler for Linux on x86 (meta-package)

REPRODUCE THE BUG:

Create a directory such as /tmp/vel and git download and compile from scratch:

git clone https://gitlab.cosma.dur.ac.uk/swift/swiftsim.git
cd swiftsim/
export MYSWIFTSIMDIR=$(pwd) ## assumes bash
autoreconf --install --symlink
./configure
make
git clone https://github.com/pelahi/VELOCIraptor-STF.git
cd VELOCIraptor-STF/stf
sed -e "s,/home/pelahi/software/swiftsim/,${MYSWIFTSIMDIR}," Makefile.config.template > Makefile.config
make

ERROR MESSAGE:

cd NBody; make make[5]: Entering directory '/tmp/vel/swiftsim/VELOCIraptor-STF/stf/NBodylib/src/NBody' mpic++ -DSINGLEPRECISION -DLONGINT -DNOMASS -DLOWPRECISIONPOS -DTREEFROGINTIDS -DSWIFTINTERFACE -DGASON -DSTARON -DBHON -c -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//Math/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//NBody/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//Analysis/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//Cosmology/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//InitCond/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//KDTree -I/include/ -L/lib/ -I/tmp/vel/swiftsim/src/ -I/tmp/vel/swiftsim/src/gravity/Default/ -I/tmp/vel/swiftsim/src/hydro/Default/ -c -o System.o System.cxx mpic++ -DSINGLEPRECISION -DLONGINT -DNOMASS -DLOWPRECISIONPOS -DTREEFROGINTIDS -DSWIFTINTERFACE -DGASON -DSTARON -DBHON -c -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//Math/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//NBody/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//Analysis/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//Cosmology/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//InitCond/ -I/tmp/vel/swiftsim/VELOCIraptor-STF/stf//NBodylib//src//KDTree -I/include/ -L/lib/ -I/tmp/vel/swiftsim/src/ -I/tmp/vel/swiftsim/src/gravity/Default/ -I/tmp/vel/swiftsim/src/hydro/Default/ -c -o Particle.o Particle.cxx Particle.cxx: In constructor ‘NBody::Particle::Particle(const Swift::gpart&, double, double, double, double, bool, double, double)’: Particle.cxx:226:7: error: ‘mass’ was not declared in this scope mass=mscale; ^~~~ Particle.cxx:233:20: error: ‘const struct Swift::gpart’ has no member named ‘potential’ gravityphi=p.potentialuscale*mass; ^~~~~ Makefile:19: recipe for target 'Particle.o' failed make[5]: [Particle.o] Error 1 make[5]: Leaving directory '/tmp/vel/swiftsim/VELOCIraptor-STF/stf/NBodylib/src/NBody' Makefile:15: recipe for target 'NBody_' failed make[4]: [NBody] Error 2 make[4]: Leaving directory '/tmp/vel/swiftsim/VELOCIraptor-STF/stf/NBodylib/src' Makefile:3: recipe for target 'all' failed make[3]: [all] Error 2 make[3]: Leaving directory '/tmp/vel/swiftsim/VELOCIraptor-STF/stf/NBodylib/src' Makefile:5: recipe for target 'all' failed make[2]: [all] Error 2 make[2]: Leaving directory '/tmp/vel/swiftsim/VELOCIraptor-STF/stf/NBodylib' Makefile:25: recipe for target 'NBodylib' failed make[1]: [NBodylib_] Error 2 make[1]: Leaving directory '/tmp/vel/swiftsim/VELOCIraptor-STF/stf' Makefile:3: recipe for target 'all' failed make: [all] Error 2

ANALYSIS/HYPOTHESES:

git log --patch NBodylib/src/NBody/Particle.cxx shows that the mass*=mscale line was added on 5 March 2018 with a swift interface update.

commit d92e98dea5e507b98e5ec859a2d26ac399d40e2b Author: pelahi <pascaljelahi ...> Date: Mon Mar 5 11:32:52 2018 +0800

Updates to interface with swift

Added extra properties to particle class, constructors and now have a fleshed out InvokeVelociraptor

Should line 78 of Particle.h with ///mass be followed by a line defining mass ? Since it seems that swiftsim has masses for all particles, maybe that's what missing? I'm a complete newbie to swift and velociraptor, so I'm just guessing.

broukema commented 6 years ago

Is there an easy way to download the full pull request as either a merge single patch, or else as a series of 22 patch files? I couldn't find any obvious way in the github web interface.

james-s-willis commented 6 years ago

I have merged the master branch into the swift-interface branch so you can just checkout the swift-interface branch. Let me know if that works.

broukema commented 6 years ago

Thanks for the fast response! It looks like this bug is solved (from a naive "does it compile?" point of view), though I get a new compile error. I'll post the new compile error as a separate bug, since I don't know enough to guess if it's related or not.

I won't close this bug because I don't know the code well enough to judge - I'm happy for someone who knows the code better (probably Pascal?) to close it.

broukema commented 6 years ago

OK, from my point of view (compilation), I no longer get any compile errors. My "new" compile error (previous comment) is because I'm used to using system-level libraries, and the LOCAL.* definitions in Makefile.config.template made me wonder whether the Makefile favours native libraries by default. In reality it seems to require hardwiring the native library include and lib directories (since there's no configure.ac file). I set ADIOSENABLE=off and

    HDF_INCL = -I/usr/include/hdf5/serial/
    HDF_LIBS = -L/usr/lib/x86_64-linux-gnu/hdf5/serial/

and make clean and make compiled fully. There don't seem to be any unit tests (e.g. make check) for checking the installation.

james-s-willis commented 6 years ago

Yes, I think VELOCIraptor looks locally for the HDF5 library. Therefore, if you want to use a specific version you have to hardcode the path in the Makefile to point to the one you want. What environment are you developing in? Does it have a module system setup? This can help with using specific library versions without hardcoding the library path in the Makefile.

broukema commented 6 years ago

As above: Debian GNU/Linux 4.9.0

dpkg -l |grep hdf5
ii  hdf5-helpers                              1.10.0-patch1+docs-3+deb9u1                 amd64        Hierarchical Data Format 5 (HDF5) - Helper tools
ii  libhdf5-100:amd64                         1.10.0-patch1+docs-3+deb9u1                 amd64        Hierarchical Data Format 5 (HDF5) - runtime files - serial version
ii  libhdf5-8:amd64                           1.8.13+docs-15+deb8u1                       amd64        Hierarchical Data Format 5 (HDF5) - runtime files - serial version
ii  libhdf5-cpp-100:amd64                     1.10.0-patch1+docs-3+deb9u1                 amd64        Hierarchical Data Format 5 (HDF5) - C++ libraries
ii  libhdf5-dev                               1.10.0-patch1+docs-3+deb9u1                 amd64        Hierarchical Data Format 5 (HDF5) - development files - serial version
ii  libhdf5-openmpi-100:amd64                 1.10.0-patch1+docs-3+deb9u1                 amd64        Hierarchical Data Format 5 (HDF5) - runtime files - OpenMPI version
ii  libhdf5-serial-dev                        1.10.0-patch1+docs-3+deb9u1                 all          transitional dummy package

The package libhdf5-8:amd64 looks like an old version which I should probably purge. The files in libhdf5-dev(amd64 architecture) are listed at https://packages.debian.org/stretch/amd64/libhdf5-dev/filelist . There are also libhdf5-openmpi-dev and libhdf5-mpich-dev available (I have libhd5-openmpi-dev installed) - presumably one of these parallel versions would have an advantage over the serial versions for any production level usage. Their files lists are at https://packages.debian.org/stretch/amd64/libhdf5-openmpi-dev/filelist and https://packages.debian.org/stretch/amd64/libhdf5-mpich-dev/filelist .

It looks to me like the hdf5 front-end compiler h5c++ should be used, in analogy to the mpi front-end mpic++. These add include and library options to the default compiler, consistently with the way that the library itself was compiled. It looks like the two cannot be used together, and the -show option for h5c++ is less convenient than the corresponding mpic++ options --showme:compile and --showme:link. However, I'm proposing a pull request https://github.com/pelahi/VELOCIraptor-STF/pull/22 for the Makefile.config.template that works for me using h5c++. This brings me to a separate issue - I haven't yet tested your Makefile.config.SWIFT-template. I'll get back to it soon...

broukema commented 6 years ago

OK, getting back to this bug - your Makefile.config.SWIFT-template works for me, without my needing to hardwire the hdf5 include and library directories, provided that I apply the following patch against your branch (this includes pull request https://github.com/pelahi/VELOCIraptor-STF/pull/22). I'm showing the patch just for clarity - my guess is that you don't really intend your Makefile.config.SWIFT-template to be added to the master branch, since I don't see any changes that would be needed by a default user.

--- Makefile.config.SWIFT-template  2018-10-22 18:18:40.799523312 +0200
+++ Makefile.config 2018-10-22 18:27:53.723258749 +0200
@@ -5,7 +5,7 @@
 #===========================================
 #type of system, where libraries are located, etc
 #===========================================
-SYSTEM="intel-standard"
+#SYSTEM="intel-standard"
 #SYSTEM="gcc-standard"
 #SYSTEM="cray"

@@ -14,10 +14,10 @@
 #===========================================
 #Parallel APIs to turn on
 #===========================================
-MPI="off"
+MPI="on"
 #reduce impact of MPI memory overhead at the cost of extra cpu cycles, suggested this be turned on
 MPIREDUCE="on"
-OMP="off"
+OMP="on"
 #===========================================

 #===========================================
@@ -25,11 +25,21 @@
 #===========================================
 LOCALGSL="on"
 LOCALFFTW="on"
-LOCALHDF="on"
+LOCALHDF="off"
 LOCALADIOS="on"
 LOCALXDR="on"
 #===========================================

+
+#===========================================
+#native (your operating system) libraries
+#===========================================
+#for using h5c++ -show to find your system level include and library files
+ifneq ($(LOCALHDF),"on")
+   HDFNATIVELIBS="on"
+endif
+#===========================================
+
 #===========================================
 #Adjust precision
 #===========================================
@@ -142,7 +152,7 @@
 #alter if necessary to point to local libraries
 #===========================================
 ifeq ($(LOCALGSL),"on")
-    GSL_DIR = /usr/local
+    GSL_DIR = 
     GSL_INCL = -I$(GSL_DIR)/include/
     GSL_LIBS = -L$(GSL_DIR)/lib/
     GSL_CONFIG =
@@ -197,8 +207,7 @@
     C+ = CC
     PARALLEL+=-DUSEMPI
 else
-    #C+ = mpiCC
-    C+ = mpicc
+    C+ = mpic++
     MPI_COMPILE_FLAGS = $(shell $(C+) --showme:compile)
     MPI_LINK_FLAGS = $(shell $(C+) --showme:link)
     PARALLEL+=-DUSEMPI $(MPI_COMPILE_FLAGS)
@@ -331,8 +340,18 @@
 ifeq ($(HDFENABLE),"on")
     C+FLAGS+= -DUSEHDF
     C+LIBS+= -lhdf5_hl_cpp -lhdf5_cpp -lhdf5_hl -lhdf5 -lz -lrt -ldl
-    IFLAGS+= $(HDF_INCL)
-    LFLAGS+= $(HDF_LIBS)
+    ifeq ($(HDFNATIVELIBS),"on")
+       H5C+ = h5c++
+       # h5c++ (version 1.10.0-patch1+docs-3+deb9u1) does not have
+       # options to show include and library options separately;
+       # awk: $1=$(NF+1) replaces the first field by a null string
+       # and $0 prints the revised string
+       IFLAGS+=$(shell $(H5C+) -show |awk '{$$1=$$(NF+1); print $$0}')
+       LFLAGS+=$(shell $(H5C+) -show |awk '{$$1=$$(NF+1); print $$0}')
+    else
+       IFLAGS+= $(HDF_INCL)
+       LFLAGS+= $(HDF_LIBS)
+   endif
 endif

 ifeq ($(ADIOSENABLE),"on")
MatthieuSchaller commented 6 years ago

FYI, the SWIFT code is still in heavy development and we can't yet guaranty stability or correct results. At this stage, I'd recommend getting in touch with the team before embarking into a simulation campaign.