petsc4py causes build problem with 64-bit PETSc

[Chris Richardson]

With 64-bit PETSc, and petsc4py, I am getting the following swig problem (see below). It appears to be an issue with the type of 64-bit integers, which on this system are typedef'd to int64_t as PetscInt. As far as I can tell, on a 64-bit system there is no difference between long int long long int and int64_t but swig seems to generate something which gcc refuses to compile...

/home/e319/e319/shared/src/dolfin/build.archer-64/dolfin/swig/modules/la/modulePYTHON_wrap.cxx: In function 'PyObject* _wrap__get_matrix_sub_vector(PyObject*, PyObject*)':
/home/e319/e319/shared/src/dolfin/build.archer-64/dolfin/swig/modules/la/modulePYTHON_wrap.cxx:38857:51: error: invalid conversion from 'PetscInt* {aka long int*}' to 'long long int*' [-fpermissive]
   ecode2 = SWIG_AsVal_PetscInt (swig_obj[1], &val2);
                                                   ^
/home/e319/e319/shared/src/dolfin/build.archer-64/dolfin/swig/modules/la/modulePYTHON_wrap.cxx:5027:1: error:   initializing argument 2 of 'int SWIG_AsVal_long_SS_long(PyObject*, long long int*)' [-fpermissive]
 SWIG_AsVal_long_SS_long (PyObject *obj, long long *val)
 ^
make[2]: *** [dolfin/swig/modules/la/CMakeFiles/_la.dir/modulePYTHON_wrap.cxx.o] Error 1
make[1]: *** [dolfin/swig/modules/la/CMakeFiles/_la.dir/all] Error 2
make: *** [all] Error 2

[Prof Garth Wells]

Probably need some fixes in the SWIG layer similar to what I did for NumPy with 32/64-bit integers.

[Jan Blechta]

Relevant files are at least

dolfin/swig/typemaps/numpy.i
dolfin/swig/typemaps/std_vector.i
 dolfin/swig/typemaps/array.i
dolfin/swig/typemaps/primitives.i

The first two seem to map la_index/PetscInt correctly to int32/int64 although I can't figure out why are there runtime checks. The latter two involve some unsigned int types which is not correct.

[Jan Blechta]

Although this does not seem related to your build problem as SWIG_AsVal_PetscInt seems to be defined in <petsc4py>/src/include/petsc4py/petsc4py.i.

[Chris Richardson]

I don't know enough about swig to suggest a fix. val2 in line 38857 of the generated modulePYTHON_wrap.cxx is defined as PETScInt. Changing this to long long int is enough to get it to compile. That is my current workaround.

[Jan Blechta]

@chris_richardson, it seems that your PETSc defines PetscInt as long. C++ does not allow automatic conversion of long* to long long* although they may be the same on a machine.

It seems to me that <petsc4py>/src/include/petsc4py/petsc4py.i assumes that 64-bit PetscInt is always long long between lines 73 and 96. Could you try reverting your fix and changing all the long longs in <petsc4py>/src/include/petsc4py/petsc4py.i to long? Although it may not be sufficient as further changes may be needed in DOLFIN but we can try.

[Prof Garth Wells]

@johanhake Could you take a look at this with your SWIG expertise?

[Jan Blechta]

I'm pretty sure that problem is that <petsc4py>/src/include/petsc4py/petsc4py.i defines PetscInt as long long if PETSC_USE_64BIT_INDICES although PETSc prefers int64_t over long long which are not the same type.

We shouldreport it upstream, shouldn't we, @johanhake ?

[Johan Hake]

I agree with @blechta here, but I do not have PETSc build with 64bit indices so I cannot really check it. But following the logic from @blechta this would be the patch to petsc4py to fix this:

diff --git a/src/include/petsc4py/petsc4py.i b/src/include/petsc4py/petsc4py.i
index 17211c1..2a960a6 100644
--- a/src/include/petsc4py/petsc4py.i
+++ b/src/include/petsc4py/petsc4py.i
@@ -77,17 +77,17 @@ Py##Pkg##_ChkErrQ($1); %set_output(VOID_Object);
           fragment=SWIG_From_frag(int))
 {
 %#if defined(PETSC_USE_64BIT_INDICES)
-%define_as(SWIG_From(PetscInt), SWIG_From(long long))
+%define_as(SWIG_From(PetscInt), SWIG_From(long))
 %#else
 %define_as(SWIG_From(PetscInt), SWIG_From(int))
 %#endif
 }
 %fragment(SWIG_AsVal_frag(PetscInt),"header",
-          fragment=SWIG_AsVal_frag(long long),
+          fragment=SWIG_AsVal_frag(long),
           fragment=SWIG_AsVal_frag(int))
 {
 %#if defined(PETSC_USE_64BIT_INDICES)
-%define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(long long))
+%define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(long))
 %#else
 %define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(int))
 %#endif

[Jan Blechta]

@johanhake It is not a proper patch just workaround, right?

[Prof Garth Wells]

Can someone report this upstream and post a link to the report here?

[Johan Hake]

I attached something that might help @chris_richardson and to shed light on how petsc4py has "solved" the issue of different sizes of PetscInt. I am not sure how a proper fix would be. For example where and when is PetscInt defined and to what different types can it be set to: int, long, long long? Is it up to the user, or is it done in the configuration of petsc? If the latter it should be easy to pick the correct type to map it to.

[Prof Garth Wells]

@johanhake Where can we find the attachment?

PETSc sets PetscInt at configure time. It's defined in petscsys.h.

[Johan Hake]

You need to look at bitbucket. It is here (there ;)) as part of my post.

[Prof Garth Wells]

@johanhake If your 'attachment' is the patch that's inlined into comment, I don't think it's a fix. Looking at petscsys,h, I see

#if defined(PETSC_HAVE_STDINT_H)
typedef int64_t Petsc64bitInt;
#elif (PETSC_SIZEOF_LONG_LONG == 8)
typedef long long Petsc64bitInt;
#elif defined(PETSC_HAVE___INT64)
typedef __int64 Petsc64bitInt;
#else
typedef unknown64bit Petsc64bitInt
#endif

#if defined(PETSC_USE_64BIT_INDICES)
typedef Petsc64bitInt PetscInt;
#else
typedef int PetscInt;
#define MPIU_INT MPI_INT
#endif

On my laptop, this means that PetscInt is in fact int64_t.

[Jan Blechta]

@johanhake It is defined here. So the patch above may not be correct from two reasons:

1. PetscInt can be int64_t, long long or __int64 (what is it?).
2. If it is int64_t is it guranteed that int64_t == long?

Where is that attachment? I'm looking to bitbucket and cannot spot anything.

[Chris Richardson]

As usual, everything is sensible, except for Windows. See:

http://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models

I believe the __int64 is also a Microsoft thing.

[Johan Hake]

Yes it was the inlined proposal to probe your problem based on the error message. Sorry for the miss leading word: attached.

Based on the snippet from Garth I am trying to compile a patch that should be more robust but SWIG and defines are not that easily resolved. I am now at the point where I need to know how to convert from/to int64_t and __int64. Do you think it would be safe to copy the conversions from/to long for int64_t?

[Prof Garth Wells]

I think we can be confident that sizeof(int64_t) >= sizeof(long). We'll need to are careful casting pointers.

If __int64 is a Windows thing, I would't bother with it. I don't see any reason why someone would need 64-bit PETSc indices on Windows.

[Johan Hake]

Ok, but here we just need to provide a mean to convert from/to int64_t and Python. But not knowing if int64_t is available was a pain to handle. Here is a suggested longer patch, which should be more robust, but pretty ugly if you ask med:

diff --git a/src/include/petsc4py/petsc4py.i b/src/include/petsc4py/petsc4py.i
index 17211c1..6dbeb69 100644
--- a/src/include/petsc4py/petsc4py.i
+++ b/src/include/petsc4py/petsc4py.i
@@ -72,22 +72,93 @@ Py##Pkg##_ChkErrQ($1); %set_output(VOID_Object);

 /* PetscInt */
 /* -------- */
+%fragment(SWIG_From_frag(int64_t), "header")
+{
+%#if defined(PETSC_HAVE_STDINT_H)
+SWIGINTERNINLINE PyObject* 
+SWIG_From_int64_t(int64_t value)
+{
+  return ((value < LONG_MIN) || (value > LONG_MAX)) ?
+    PyLong_FromLongLong(value) : PyLong_FromLong(static_cast< long >(value)); 
+}
+%#else
+SWIGINTERNINLINE PyObject* 
+SWIG_From_int64_t(long value)
+{
+  return PyLong_FromLong(value);
+}
+%#endif
+}
+
+%fragment(SWIG_AsVal_frag(int64_t), "header")
+{
+%#if defined(PETSC_HAVE_STDINT_H)
+SWIGINTERN int
+SWIG_AsVal_int64_t (PyObject *obj, int64_t* val)
+{
+  if (PyInt_Check(obj)) {
+    if (val) *val = PyInt_AsLong(obj);
+    return SWIG_OK;
+  } else if (PyLong_Check(obj)) {
+    int64_t v = PyLong_AsLong(obj);
+    if (!PyErr_Occurred()) {
+      if (val) *val = v;
+      return SWIG_OK;
+    } else {
+      PyErr_Clear();
+    }
+  }
+ }
+%#else
+SWIG_AsVal_int64_t (PyObject *obj, long* val)
+{
+  if (PyInt_Check(obj)) {
+    if (val) *val = PyInt_AsLong(obj);
+    return SWIG_OK;
+  } else if (PyLong_Check(obj)) {
+    long v = PyLong_AsLong(obj);
+    if (!PyErr_Occurred()) {
+      if (val) *val = v;
+      return SWIG_OK;
+    } else {
+      PyErr_Clear();
+    }
+  }
+ }
+%#endif
+}
 %fragment(SWIG_From_frag(PetscInt),"header",
+          fragment=SWIG_From_frag(int64_t),
           fragment=SWIG_From_frag(long long),
           fragment=SWIG_From_frag(int))
 {
+
 %#if defined(PETSC_USE_64BIT_INDICES)
-%define_as(SWIG_From(PetscInt), SWIG_From(long long))
+  %#if defined(PETSC_HAVE_STDINT_H)
+  %define_as(SWIG_From(PetscInt), SWIG_From(int64_t))
+  %#elif (PETSC_SIZEOF_LONG_LONG == 8)
+  %define_as(SWIG_From(PetscInt), SWIG_From(long long))
+  %#else
+  %define_as(SWIG_From(PetscInt), SWIG_From(unknown64bit))
+  %#endif
 %#else
 %define_as(SWIG_From(PetscInt), SWIG_From(int))
 %#endif
 }
+
 %fragment(SWIG_AsVal_frag(PetscInt),"header",
+          fragment=SWIG_AsVal_frag(int64_t),
           fragment=SWIG_AsVal_frag(long long),
           fragment=SWIG_AsVal_frag(int))
 {
 %#if defined(PETSC_USE_64BIT_INDICES)
-%define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(long long))
+  %#if defined(PETSC_HAVE_STDINT_H)
+  %define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(int64_t))
+  %#elif (PETSC_SIZEOF_LONG_LONG == 8)
+  %define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(long long))
+  %#else
+  %define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(unknown64bit))
+  %#endif
 %#else
 %define_as(SWIG_AsVal(PetscInt), SWIG_AsVal(int))
 %#endif

[Jan Blechta]

Moreover sizeof(long long) >= sizeof(int64_t) == 64, see Wikipedia long long.

[Prof Garth Wells]

@johanhake I think we can reasonably assume that int64_t is available on systems that have a 64 bit build of PETSc.

[Johan Hake]

I agree @garth-wells but I have not figured out how to access information about what petsc uses when the SWIGing is done. We had the same problem when we safeguarded the SWIG layer for different types of dolfin::la_int. That is why the patch is so ugly. We need to generate code for the different scenarios and then decide what code is going to be used using (C++) compile time ifdefs.

We could determine via CMake what petsc uses, we have discussed this before, and then set some SWIG aware defines, but I am not sure this is doable for 3d party SWIG code, like petsc4py.i.

[Jan Blechta]

Reported upstream. So watch and vote!

[Jan Blechta]

@johanhake, actually why can't problem be fixed by following the same ifdef pattern in <petsc4py>/src/include/petsc4py/petsc4py.i like in petscsys.h, lines 216-248? You can check all the macros PETSC_HAVE_STDINT_H, PETSC_SIZEOF_LONG_LONG == 8, can't you? Is the problem that you don't know how to work with int64_t in SWIG?

[Johan Hake]

@blechta the problem is that we do not have the defines available when we do the SWIGing. So we need to (as is done in the original petsc4py.i file) include code for the different possibilities and enclose these within ifdefs. Eventually these will be checked with the correct definitions from petscsys.h when the generated code is compiled.

We have discussed to acquire this information in during configuration in CMakeLists.txt and then pass that information to the swig comands as swig -DSOME_DEFINE. It will then be available during SWIGing, but we cannot make any such assumptions for code included in a 3rd party library as petsc4py.

[Chris Richardson]

This needs to be fixed upstream in petsc4py

[Lisandro Dalcin]

https://bitbucket.org/petsc/petsc4py/branch/dalcinl/swig-int64

[Jan Blechta]

@dalcinl Lets DOLFIN compile with 64-bit PETSc indices (together with other fix in DOLFIN branch jan/fix-mumpslusolver-int).

@chris_richardson Can you try if it works?