daverio / latfield2 Goto Github PK

Reading Latfield2's paper you will find on page 6 a code snippet

Field<double> phi(lat,comp);

suggesting to readers that Latfield provides an interface that allows object initialization upon construction. That's fine, this is one of features of C++ that C doesn't have.
On the other hand one of the examples provided in this repository shows that a complex lattice is initialized by calling the method Lattice::initializeRealFFT, see here.

A user, like myself, might think that the dual lattice might as well be initialized upon construction, just like Field does, and can try the following code:

#include <iostream>
#define FFT3D
#include "LATfield2.hpp"

using LATfield2::Lattice;
int main()
{
    const int n=2,m=2;
    const int BoxSize = 64;
    const int dim =3;
    const int comp = 1;
    const int halo = 1;
    
    parallel.initialize(n,m);
    Lattice lat(dim,BoxSize,halo);
    
    Lattice latK(lat);
    //latK.initializeRealFFT(lat,halo);
    return 0;
}

which compiles.

$ make
g++ main.cpp -I../.. `pkg-config --cflags --libs fftw3 ompi`

However, that code fails at runtime:

$ mpirun -np 4 a.out 
free(): double free detected in tcache 2

This happens because the developers didn't write a copy constructor for Lattice, and the C++ compiler did it automatically.
The fact is that some members of Lattice are raw pointers to allocated memory owned by an instance of Lattice.
Copy will lead to double freed memory and thus it should not be allowed!

In this library you can find global values like this

int symmetric = 1;
int unsymmetric = 0;

see here for instance.
These two for instance represent two possible types of field symmetric or asymmetric.
But they should be declared const since they are not meant for writing.

Furthermore their purpose is nicely handled by a C++ feature called enum class, see Stroustrup The C++ Programming Language section 8.4.1. An enum class is a scoped and strongly typed enumeration.
That means by writing:

enum class symmetry_t {
    symmetric, unsymmetric
};

1. you get them to be read only objects by default,
2. they do not clash with other names, symmetric and unsymmetric are within the scope symmetry_t:: and not global,
3. you get type check at compile time,
4. you can have templated classes and functions on the symmetry_t type, for example:

template<symmetry_t>
class Field;

Some examples from the book read like this:

symmetry_t s1 = 7 ;// error: int->symmetry_t conversion
int s2 = symmetric; // error: symmetric is not in scope
int s3 = symmetry_t::symmetric; // error: no symmetry_t->int conversion
symmetry_t s4 = symmetry_t::symmetric; // OK

That is you field constructor

		Field(Lattice& lattice, int rows, int cols, int symmetry=unsymmetric);

could become

		Field(Lattice& lattice, int rows, int cols, symmetry_t symmetry = symmetry_t::unsymmetric);

and from the user point of view you couldn't make

Field<Real> phi(lat,row,cols, 2 ); // error: int->symmetry_t conversion

which in any case is a God bless compiler error because who knows what 2 stands for?
Instead, the user is bound to type:

Field<Real> phi(lat,row,cols, symmetry_t::symmetric );

LATfield cannot be used in a project with multiple translation units.
For instace consider this little project with the following files

# makefile
EXE := hello
OBJS := main.o myfun.o
INCLUDE := -I../LATfield2 $(shell pkg-config --cflags ompi)
LIBS := $(shell pkg-config --libs ompi)

default: $(EXE)

$(EXE) : $(OBJS)
	c++ $(OBJS) -o $@ $(LIBS)

$(OBJS) : %.o : %.cpp
	c++ -c $^ $(INCLUDE) $(LIBS)

clean : 
	-rm $(OBJS) $(EXE)

.PHONY: clean default

// main.cpp
#include "myfun.hpp"
#include <mpi.h>
#include "LATfield2.hpp"

int main()
{
    parallel.initialize(2,2);
    hello();
    return 0;
}

// myfun.hpp
#pragma once

void hello();

// myfun.cpp
#include "myfun.hpp"
#include <iostream>
#include "LATfield2.hpp"

void hello()
{
    std::cout << "Hello\n";
}

If the line #include "LATfield2.hpp" is commented in the myfun.cpp the project compiles. Otherwise you get a linker error:

/usr/bin/ld: myfun.o: in function `Parallel2d::Parallel2d()':
myfun.cpp:(.text+0x0): multiple definition of `Parallel2d::Parallel2d()'; main.o:main.cpp:(.text+0x0): first defined here

It is very odd the way particles' masses are handled in LATfield2 (and gevolution, because of LATfield).
If I understood correctly: there is a macro called CREATE_MEMBER_DETECTOR that defines a class to check for member variables using the principle of "Substitution Failure Is Not An Error" (SFINAE in C++ jargon). The idea should be that if the particle class has the mass variable then that mass is used, otherwise the mass of the particle-info class is used.

What strikes me is to see lines of code like this:

if (parts->mass_type() == GLOBAL_MASS )
{
 mass = *(double*)((char*)parts->parts_info() + offset_mass);
}

or

if(parts->mass_type() ==FROM_PART)
 {
      mass = *(double*)((char*)&(*it)+offset_mass);
 }

I mean, the purpose of using C++ features is to make the programmers work easier, not harder.
Couldn't you find a way to construct an interface that allows to write

if (parts->mass_type() == GLOBAL_MASS )
{
 mass = parts->parts_info().mass;
}

or

if(parts->mass_type() ==FROM_PART)
 {
      mass = it->mass;
 }

?

If I compile this empty program, with the latfield header, it crashes:

#include "LATfield2.hpp"

int main (int argc, char ** argv ) {
  parallel.initialize(1, 1);

  std::cout << "Hello and goodbye world.\n";
  return 0;
}

output:

./test.out
Hello and goodbye world.
[macth83:15594] *** Process received signal ***
[macth83:15594] Signal: Segmentation fault: 11 (11)
[macth83:15594] Signal code:  (0)
[macth83:15594] Failing at address: 0x0
[macth83:15594] [ 0] 0   libsystem_platform.dylib            0x00007fff9c5f0eaa _sigtramp + 26
[macth83:15594] [ 1] 0   ???                                 0x0000000000000000 0x0 + 0
[macth83:15594] [ 2] 0   test.out                            0x0000000100f6170c _ZN10Parallel2dD1Ev + 28
[macth83:15594] [ 3] 0   libsystem_c.dylib                   0x00007fff8d25846b __cxa_finalize_ranges + 345
[macth83:15594] [ 4] 0   libsystem_c.dylib                   0x00007fff8d25876f exit + 55
[macth83:15594] [ 5] 0   libdyld.dylib                       0x00007fffa087d5b4 start + 8
[macth83:15594] *** End of error message ***
Segmentation fault: 11

Hi David

I have a error when using the newest version of HDF5.

Warning! HDF5 library version mismatched error
The HDF5 header files used to compile this application do not match
the version used by the HDF5 library to which this application is linked.
Data corruption or segmentation faults may occur if the application continues.
This can happen when an application was compiled by one version of HDF5 but
linked with a different version of static or shared HDF5 library.
You should recompile the application or check your shared library related
settings such as 'LD_LIBRARY_PATH'.
You can, at your own risk, disable this warning by setting the environment
variable 'HDF5_DISABLE_VERSION_CHECK' to a value of '1'.
Setting it to 2 or higher will suppress the warning messages totally.
Headers are 1.8.12, library is 1.8.18
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<

Best regards
Song

The following innocent looking code crashes:

#define FFT3D
#include <fftw3.h>
#include "LATfield2.hpp"

using namespace LATfield2;

int main()
{
    const int n=2,m=2;
    const int BoxSize = 64 ;
    const int halo = 1;
    const int dim = 3;
    const int comp = 1;

    parallel.initialize(n,m);
    Lattice lat(dim,BoxSize,halo);
    Site x(lat);
    Field<Real> phi;
    phi.initialize(lat,comp);
    
    //Lattice latK;
    //latK.initializeRealFFT(lat, halo);
    //rKSite k(latK);
    //Field<Imag> phiK;
    //phiK.initialize(latK,comp);
    //
    //PlanFFT<Imag> planPhi(&phi,&phiK);

    for (x.first(); x.test(); x.next())
    {
        phi(x) = 1.0;
    }
}

However, it doesn't when the commented lines are uncommented.

This is the makefile used to compile:

default: main
LATFIELD:=../..

main : main.cpp
	g++ main.cpp -I${LATFIELD} `pkg-config --cflags --libs fftw3 ompi`

and it can be executed with 4 MPI processes:

$ mpirun -np 4 a.out

Feel free to close this if inappropriate, but I thought you might like to know that the website www.latfield.org no longer hosts the LATfield2 documentation. The documentation can still be generated using doxygen from the source code, but as the ArXiv paper and other resources link this domain I thought it might be worth notifying you.

LATfield2 is a header only library and it contains using directives all over.
Refer to Stroustrup's The C++ Programming Language 4th edition, Chapter 14 Namespaces and section 15.2.2 Header Files
as to why this is a bad practice.

This code for instance compiles:

#include <iostream>
#include "LATfield2.hpp"

int main()
{
    cout << "Namespace pollution\n";    
    return 0;
}

The user's namespace has been polluted without consent with std.

LATfield2 commit 1ad5d43 and earlier has a stack buffer overflow.

Compile the minimal example below with the usual LATfield2 options and:

      -fstack-protector -fsanitize=address

to see the stack-buffer-overflow error.

You might need to make 'str_filename' longer on your
machine to detect the bug. This presumably depends on the stack size
on your machine, or other machine-dependent implementation details.

Do a git checkout f27070f, recompile, run, and there should no
longer be a stack-buffer-overflow error. :)

TODO: There will still be quite a few memory leaks detected, which are
probably worth tidying up...

#include "LATfield2.hpp"
using namespace LATfield2;

int main(int argc, char **argv)
{
    parallel.initialize(2,2);
    int dim = 3;
    int latSize[3] = {2,2,2};
    int halo = 1;
    Lattice lat(dim,latSize,halo);
    Field<Real> phi;
    phi.initialize(lat);
    phi.alloc();
    string str_filename = "12345678901234567";
    phi.saveHDF5(str_filename);
}