search

coin-or / Ipopt

COIN-OR Interior Point Optimizer IPOPT
https://coin-or.github.io/Ipopt
Other
1.36k stars 272 forks source link

Recent problem with ThirdParty-Mumps #479

Closed mottelet closed 3 years ago

mottelet commented 3 years ago

Hi, I had no problem building Ipopt (on Linux and macOS with MUMPS only, with the script given at the end of this post) since april but yesterday I saw that the configure script of ThirdParty-Mumps fails on macOS (I use gcc8.3) :

configure:18900: gcc -o conftest -O2 -DNDEBUG  -I/Users/mottelet/git/sci-ipopt/thirdparty/Darwin/include/coin-or/metis/ -L/Users/mottelet/git/sci-ipopt/thirdparty/Darwin/lib conftest.c -lcoinmetis >&5
conftest.c:40:5: error: #error "Metis does not use int type for idx_t, but this build of MUMPS requires it"
    #error "Metis does not use int type for idx_t, but this build of MUMPS requires it"
     ^~~~~
conftest.c: In function 'main':
conftest.c:58:4: error: too few arguments to function 'metis_nodend'
    metis_nodend((int*)0, (int*)0, (int*)0, (int*)0, (int*)0, (int*)0);
    ^~~~~~~~~~~~
In file included from /Users/mottelet/git/sci-ipopt/thirdparty/Darwin/include/coin-or/metis/metis.h:36,
                 from conftest.c:35:
/Users/mottelet/git/sci-ipopt/thirdparty/Darwin/include/coin-or/metis/proto.h:85:6: note: declared here
 void metis_nodend(int *, idxtype *, idxtype *, int *, int *, idxtype *, idxtype *);

Did something change somewhere (I had no change on my macOS or gcc) ?

#!/bin/bash

git clone -b stable/3.13 https://github.com/coin-or/Ipopt.git
git clone -b stable/2.0 https://github.com/coin-or-tools/ThirdParty-Metis.git
git clone -b stable/2.1 https://github.com/coin-or-tools/ThirdParty-Mumps.git

cd ..
PREFIX=$(pwd)/$(uname)
LIB=$PREFIX/lib
rm -rf LIB/*
cd build

cd ThirdParty-Metis
./get.Metis
./configure --prefix=$PREFIX --enable-static=yes --enable-shared=no
make -j
make install

cd ../ThirdParty-Mumps
./get.Mumps
LDFLAGS=-L$LIB ./configure --prefix=$PREFIX --enable-static=yes --enable-shared=no --with-metis --with-metis-cflags=-I$PREFIX/include/coin-or/metis/ --with-metis-lflags=-lcoinmetis
make -j
make install

cd ../Ipopt
LDFLAGS=-L$LIB ./configure --prefix=$PREFIX --enable-static=yes --enable-shared=no --without-asl --without-hsl --with-mumps --with-mumps-cflags=-I$PREFIX/include/coin-or/mumps/ --with-mumps-lflags=-lcoinmumps
make -j
make install
svigerske commented 3 years ago

A check was added with coin-or-tools/ThirdParty-Mumps@787c1ac1 to check that the integer type used in Metis matches the one used in MUMPS (triggered by coin-or-tools/ThirdParty-HSL#8). This broke compatibility with Metis 4. Should be fixed now. Thank you.

mottelet commented 3 years ago

OK, but I still do not understand why this affected Mumps stable/2.1 branch (which is supposed to be freezed, no ?). Here the fix you just commited is in master.

mottelet commented 3 years ago

So just tell me which stable versions of Metis+Mumps I can rely on.

svigerske commented 3 years ago

The branch isn't freezed, it just doesn't get larger feature updates. Releases would be fixed, but we haven't done any for a while.

Adding the check for the integer type in Metis was meant to be a small feature update, sorry that it had a bug. I fixed this in stable/2.1 and stable/3.0 (=master).

ThirdParty-Metis is no longer updated. Since 3.13.2, the Ipopt installation instructions suggest using the systems package manager to get a current Metis.

If you want MUMPS 4, use branch stable/2.1 of ThirdParty-MUMPS. If you want MUMPS 5, use branch stable/3.0.

mottelet commented 3 years ago

Does the fix you did 2 hours ago (https://github.com/coin-or-tools/ThirdParty-Mumps/commit/a071bb0c2bc9dc31a43d8197bb02083fdb8e1fce) is supposed to allow building master branch of ThirdParty-MUMPS above stable/2.0 branch of Thridparty-Metis ? Because I just tried and I still have the same problem (failure of the configure script).

svigerske commented 3 years ago

Yes, should be. Can you check that your checkout of master matches the one from coin-or-tools/ThirdParty-Mumps? If so, check what the failure in config.log is now.

mottelet commented 3 years ago

Yes, should be. Can you check that your checkout of master matches the one from coin-or-tools/ThirdParty-Mumps? If so, check what the failure in config.log is now.

portmottelet-hz-2:ThirdParty-Mumps mottelet$ git log -n1
commit a071bb0c2bc9dc31a43d8197bb02083fdb8e1fce (HEAD -> master, origin/stable/3.0, origin/mumps5, origin/master, origin/HEAD)
Author: Stefan Vigerske <svigerske@gams.com>
Date:   Fri May 28 15:11:04 2021 +0200

    fix that IDXTYPEWIDTH is not defined with Metis 4

    - translate IDXTYPE_INT being defined or not to IDXTYPEWIDTH being 4 or 8
    - fixes coin-or/Ipopt#479

and configure fails with

configure:18910: gcc -o conftest -O2 -DNDEBUG  -I/Users/mottelet/git/sci-ipopt/thirdparty/Darwin/include/coin-or/metis/ -L/Users/mottelet/git/sci-ipopt/thirdparty/Darwin/lib conftest.c -lcoinmetis >&5
conftest.c:45:6: error: #error "Metis does not use int type for idx_t, but this build of MUMPS requires it"
     #error "Metis does not use int type for idx_t, but this build of MUMPS requires it"
svigerske commented 3 years ago

It worked for me. Can you debug this a bit?

The code to be compiled is

   ...   /// check your config.log for what is here
   #ifdef __STDC__
    #include <limits.h>
   #endif
   #include "metis.h"
   #ifndef METIS_VER_MAJOR
    #define METIS_VER_MAJOR 4
    #ifdef IDXTYPE_INT
     #define IDXTYPEWIDTH 4
    #else
     #define IDXTYPEWIDTH 8
    #endif
   #endif
   #if defined(MUMPS_INTSIZE32) && defined(INT_WIDTH) && INT_WIDTH != IDXTYPEWIDTH
    #error "Metis does not use int type for idx_t, but this build of MUMPS requires it"
   #endif
   #if defined(MUMPS_INTSIZE64) && IDXTYPEWIDTH != 8
    #error "Metis does not use 64-bit integers for idx_t, but this build of MUMPS requires it"
   #endif

With Metis 4, metis.h does not define METIS_VER_MAJOR. metis.h includes struct.h and struct.h has

#define IDXTYPE_INT

This should lead to

     #define IDXTYPEWIDTH 4

in the code above.

MUMPS_INTSIZE32 should be defined. With that, the condition

   #if defined(MUMPS_INTSIZE32) && defined(INT_WIDTH) && INT_WIDTH != IDXTYPEWIDTH

should not hold, because INT_WIDTH is 4, but also IDXTYPEWIDTH is 4.

mottelet commented 3 years ago

I have in config.log

#ifdef __STDC__
|     #include <limits.h>
|    #endif
|    #include "metis.h"
|    #ifndef METIS_VER_MAJOR
|     #define METIS_VER_MAJOR 4
|     #ifdef IDXTYPE_INT
|      #define IDXTYPEWIDTH 4
|     #else
|      #define IDXTYPEWIDTH 8
|     #endif
|    #endif
|    #if defined(MUMPS_INTSIZE32) && defined(INT_WIDTH) && INT_WIDTH != IDXTYPEWIDTH
|     #error "Metis does not use int type for idx_t, but this build of MUMPS requires it"
|    #endif
|    #if defined(MUMPS_INTSIZE64) && IDXTYPEWIDTH != 8
|     #error "Metis does not use 64-bit integers for idx_t, but this build of MUMPS requires it"
|    #endif

and metis.h is

/*
 * Copyright 1997, Regents of the University of Minnesota
 *
 * metis.h
 *
 * This file includes all necessary header files
 *
 * Started 8/27/94
 * George
 *
 * $Id: metis.h,v 1.1 1998/11/27 17:59:21 karypis Exp $
 */

#include <stdio.h>
#ifdef __STDC__
#include <stdlib.h>
#else
#include <malloc.h>
#endif
#include <strings.h>
#include <string.h>
#include <ctype.h>
#include <math.h>
#include <stdarg.h>
#include <time.h>

#ifdef DMALLOC
#include <dmalloc.h>
#endif

#include <defs.h>
#include <struct.h>
#include <macros.h>
#include <rename.h>
#include <proto.h>
mottelet commented 3 years ago

If you want to test, the files of Scilab interface to ipopt are @ https://gitlab.com/esi-group/scilab/forge/sci-ipopt/. The thirdparty (Metis, Mumps, Ipopt) build script https://gitlab.com/esi-group/scilab/forge/sci-ipopt/-/blob/master/thirdparty/build/build.sh is checking out Mumps stable/2.1 branch but replacing by master branch does not change the problem. I tested under Linux and macOS.

svigerske commented 3 years ago

Not really. As said, it would be nice if you could just debug a bit. I outlined the logic of the test. With a bit of editing in configure (around line 18813), it shouldn't be difficult to figure out what is going wrong on your system.

mottelet commented 3 years ago

What am I supposed to do (I never debugged a configure script) ? How do I insert something to print the values of constants (this part of conftest.c is before the main) ?

svigerske commented 3 years ago

configure just compiles the C code that you see there.

Adding this would print the GCC version, for example:

#define XSTR(x) STR(x)
#define STR(x) #x
#pragma message "__GNUC__ =" XSTR(__GNUC__)
mottelet commented 3 years ago

That's what I presumed after

portmottelet-hz-2:ThirdParty-Mumps mottelet$ gcc-fsf-8 -dM -E - < /dev/null | grep INT_WIDTH
#define __INT_WIDTH__ 32
#define __WINT_WIDTH__ 32

In fact INT_WIDTH is 32 on my system (gcc8)

conftest.c:46:9: note: #pragma message: __GNUC__ =8
 #pragma message "__GNUC__ =" XSTR(__GNUC__)
         ^~~~~~~
conftest.c:47:9: note: #pragma message: INT_WIDTH =32
 #pragma message "INT_WIDTH =" XSTR(INT_WIDTH)
mottelet commented 3 years ago

I replaced 4 by 32 in the configure script but the next failure is:

conftest.c: In function 'main':
conftest.c:67:4: error: too few arguments to function 'metis_nodend'
    metis_nodend((int*)0, (int*)0, (int*)0, (int*)0, (int*)0, (int*)0);
    ^~~~~~~~~~~~
In file included from /Users/mottelet/git/sci-ipopt/thirdparty/Darwin//include/coin-or/metis/metis.h:36,
                 from conftest.c:35:
/Users/mottelet/git/sci-ipopt/thirdparty/Darwin//include/coin-or/metis/proto.h:85:6: note: declared here
 void metis_nodend(int *, idxtype *, idxtype *, int *, int *, idxtype *, idxtype *);
mottelet commented 3 years ago

After fixing the prototype in the configure, then the following compilation error occurs:

MUMPS/src/mumps_metis_int.c: In function ‘mumps_metis_idxsize_’:
MUMPS/src/mumps_metis_int.c:36:24: error: ‘IDXTYPEWIDTH’ undeclared (first use in this function); did you mean ‘IDXTYPE_INT’?
        *metis_idx_size=IDXTYPEWIDTH;
svigerske commented 3 years ago

OK, can you try again with ThirdParty-Mumps/master?

I added another check whether it is Metis 4, in which case MUMPS is now build with -Dmetis4 instead of -Dmetis. There should be a line in configure saying

checking Metis version... 4

ThirdParty-Mumps/stable/2.x was using another mechanism to allow to use Metis 5 with MUMPS 4, and something got messed up when moving to MUMPS 5.

PS: Also fixed the wrong unit for IDXTYPEWIDTH and the wrong prototype for metis_nodend. Still wondering whether there was actually something that was not wrong.

mottelet commented 3 years ago

OK I just tried (sorry for the delay (familiy lunch is sacred, eh ?)), and everything works again !

S.

svigerske commented 3 years ago

Thank you (and of course lunch should always go first, or dinner, or breakfast).
I updated also branches stable/3.0, mumps5, and stable/2.1 of ThirdParty-Mumps now. If you want to go back to stable/2.1, maybe you can give that a try.

mottelet commented 3 years ago

Thanks Stefan it works again with stable/2.1 branch of ThirdParty-Mumps.

svigerske commented 3 years ago

OK, good. I tagged the current versions as releases, so you can use that if you want something fixed.