Add a meson build system #141

amontoison · 2023-09-21T02:53:18Z

close #129

I did the Meson build system tonight.

During the compilation I have an error because we need an header config.h that is
generated from configure.ac by autoheader.
We need to solve #135.

The Meson build system will also help to solve conda-forge/spral-feedstock#2.

jfowkes · 2023-09-21T08:35:30Z

Excellent many thanks, I think initially we just need to set in config.h (see #135):

HAVE_HWLOC -- used to detect NUMA regions
SPRAL_HAVE_METIS_H -- used to define METIS5 32bit/64bit index types
SIZEOF_IDX_T -- used to define METIS5 32bit/64bit index types
HAVE_NVCC -- used to build CUDA kernels
HAVE_SCHED_GETCPU -- used to check if sched_getcpu() can be used

although I would like to find a better way to detect METIS5 64bit index types (see #136).

Can we autodetect the METIS header with meson and check sizeof(idx_t)?

amontoison · 2023-09-21T18:10:36Z

Can we autodetect the METIS header with meson and check sizeof(idx_t)?

We are already autodetecting the METIS header for libHSL:

libmetis_name = get_option('libmetis')
libmetis_path = get_option('libmetis_path')
libmetis_include = include_directories(get_option('libmetis_include'))
libmetis_version = get_option('libmetis_version')

# C and Fortran compilers
cc = meson.get_compiler('c')
fc = meson.get_compiler('fortran')

# Dependencies
libmetis = fc.find_library(libmetis_name, dirs : libmetis_path, required : false)
has_metish = cc.has_header('metis.h', include_directories : libmetis_include)

We can probably check the content of metis.h to check if we have

#define IDXTYPEWIDTH 32

or

#define IDXTYPEWIDTH 64

In the same time, we can also check if we have METIS 4 or METIS 5 with METIS_VER_MAJOR.

amontoison · 2023-10-01T22:06:04Z

@jfowkes
We have a working Meson build system now.
HAVE_CONFIG_H was missing in some C++ files, I modified them such we can compile SPRAL with Meson.
A few comments:

The current build system can't handle the GPU version of SPRAL. I propose to do that when the CPU version is working well.
CirrusCI is not activated for the repository ralna/spral, can you add it such that we use CI on FreeBSD and Apple-M1 platforms?
I want to add an option ssids_only (or another name) such that only ssids is compiled for Ipopt users, what do you think? It's also relevant for GALAHAD.
For the options in config.h, I found a way to add dynamically HAVE_HWLOC and SPRAL_HAVE_METIS_H.
SIZEOF_IDX_T is generated by metis5_wrappers.F90 for us if SPRAL_HAVE_METIS_H is found but we can add an option
for the users to modify it directly.
HAVE_NVCC should be easy to add when we will work on GPU support.
For HAVE_SCHED_GETCPU, I don't how we can check if sched_getcpu() is available, any idea?
The tests related to ssmfe requires CBLAS, we should update it to directly use Fortran routines. I pushed the file cblas.h as hotfix.

amontoison · 2023-10-02T07:44:39Z

It's finally working! 🎉 🎉 🎉

Update: The logs for Windows: https://github.com/ralna/spral/suites/16773345740/artifacts/957859756

Test failure at ../tests/ssids/kernels/block_ldlt.cxx:221
ASSERT_EQ(rloc1, rloc2) failed
[(test_maxloc_torture<double, 128>(10000))[fail]]

jfowkes · 2023-10-02T08:58:38Z

@amontoison excellent many thanks! We are able to reproduce #144 from @bharswami so I will now file that as a bug. Would be nice to fix this before merging the meson build system.

jfowkes · 2023-10-03T10:17:23Z

@amontoison in answer to your comments:

Is there a reason you ditch hwloc on Windows and Mac? We have hwloc working fine on Mac in the current CI tests...
Happy to deal with GPU meson build once we have CPU meson build working
I have activated CirrusCI for ralna/spral
I also want an ssids_only option since 99% users only want SSIDS
For sched_get_cpu() this is in glibc (The C standard library) on Linux, so we can set HAVE_SCHED_GETCPU to true on Linux platforms, not sure about Mac or Windows?
I agree that SSMFE should not be relying on CBLAS but Fortran BLAS instead

amontoison · 2023-10-03T11:11:14Z

Is there a reason you ditch hwloc on Windows and Mac? We have hwloc working fine on Mac in the current CI tests...

A header file hwloc.h is missing if I try to compile on Mac and Windows. I compiled without hwloc just to check for other errors.

Happy to deal with GPU meson build once we have CPU meson build working.

Great!

I have activated CirrusCI for ralna/spral

Ok, I will try to compile SPRAL on FreeBSD and Apple M1.

I also want an ssids_only option since 99% users only want SSIDS

Ok I will add it today.

For sched_get_cpu() this is in glibc (The C standard library) on Linux, so we can set HAVE_SCHED_GETCPU to true on Linux platforms, not sure about Mac or Windows?

How HAVE_SCHED_GETCPU is set by the autotools build system?

I agree that SSMFE should not be relying on CBLAS but Fortran BLAS instead

Let's open an issue about that and update SSMFE in another PR.

jfowkes · 2023-10-03T12:49:37Z

A header file hwloc.h is missing if I try to compile on Mac and Windows. I compiled without hwloc just to check for other errors.

You just need to point Meson to the directory with hwloc.h, on Mac you can use brew --prefix.

How HAVE_SCHED_GETCPU is set by the autotools build system?

It includes <sched.h> (from GLIBC or equivalent) and then tries to compile sched_getcpu():

AC_TRY_LINK([#include <sched.h>], [sched_getcpu();],

More details here: https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Obsolete-Macros.html

meson.build

amontoison · 2023-10-04T18:03:59Z

@jfowkes
I have this error on FreeBSD.

   Current memory used:         416 bytes
   Maximum memory used:         416 bytes
***Memory allocation failed for OMETIS: cptr. Requested size: 10428522542804238376 bytes

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

Backtrace for this error:
#0  0x823820369 in ???
#1  0x82381f495 in ???
#2  0x82a913b5f in handle_signal
	at /usr/src/lib/libthr/thread/thr_sig.c:303
#3  0x82a91311e in thr_sighandler
	at /usr/src/lib/libthr/thread/thr_sig.c:246
#4  0x7ffffffff8a2 in ???
#5  0x822611240 in __spral_metis_wrapper_MOD_metis_order32
	at ../src/metis5_wrapper.F90:170

I checked what we should not compile if we want to add an option ssids_only and the difference is very small. We will not compile lsmr and ssmfe in that case. The issue is that we need to modify the header file spral.h to not include lsmr.h and ssmfe.h.

I just need to add the HAVE_SCHED_GETCPU in Meson to finalize the PR. (done ✔️ )

jfowkes · 2023-10-05T08:54:29Z

@jfowkes I have this error on FreeBSD.

   Current memory used:         416 bytes
   Maximum memory used:         416 bytes
***Memory allocation failed for OMETIS: cptr. Requested size: 10428522542804238376 bytes

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

Backtrace for this error:
#0  0x823820369 in ???
#1  0x82381f495 in ???
#2  0x82a913b5f in handle_signal
	at /usr/src/lib/libthr/thread/thr_sig.c:303
#3  0x82a91311e in thr_sighandler
	at /usr/src/lib/libthr/thread/thr_sig.c:246
#4  0x7ffffffff8a2 in ???
#5  0x822611240 in __spral_metis_wrapper_MOD_metis_order32
	at ../src/metis5_wrapper.F90:170

@amontoison I see errors like this when using 64bit integers but METIS5 is built with 32bit integer support and vice versa. Is the FreeBSD METIS 64bit integer? Or is Meson not correctly detecting the METIS headers and the SIZEOF_IDX_T on FreeBSD?

I checked what we should not compile if we want to add an option ssids_only and the difference is very small. We will not compile lsmr and ssmfe in that case. The issue is that we need to modify the header file spral.h to not include lsmr.h and ssmfe.h.

Okay let's skip an SSIDS only build for now, seems like it's not worth it atm.

I just need to add the HAVE_SCHED_GETCPU in Meson to finalize the PR. (done ✔️ )

Great stuff!

amontoison · 2023-10-05T09:09:29Z

Oh, I throught that SIZEOF_IDX_T was a variable of metis.h but it's not the case.

I propose to replace the line https://github.com/ralna/spral/blob/master/src/metis5_wrapper.F90#L23 with
#if IDXTYPEWIDTH == 64 directly.
IDXTYPEWIDTH is defined by the metis header.

jfowkes · 2023-10-05T09:29:24Z

Oh, I throught that SIZEOF_IDX_T was a variable of metis.h but it's not the case.

I propose to replace the line https://github.com/ralna/spral/blob/master/src/metis5_wrapper.F90#L23 with #if IDXTYPEWIDTH == 64 directly. IDXTYPEWIDTH is defined by the metis header.

Yeah sure, that seems sensible to me. Why wasn't this done in the first place? We should also add some tests where we test against 64bit integer METIS so we know it woks?

jfowkes · 2023-10-05T10:58:22Z

@amontoison also you will have to add CBLAS support for Meson, some of the SSMFE C examples require CBLAS functions for various matrix modification routines, eg:

 for(int i=0; i<rci.nx; i++) {
    if( rci.kx == rci.ky ) {
       double s = cblas_dnrm2(n, &W[rci.kx][rci.jx+i][0], 1);
       if( s > 0 )
          cblas_dscal(n, 1/s, &W[rci.kx][rci.jx+i][0], 1);
    } else {
       double s = sqrt(fabs(cblas_ddot(
          n, &W[rci.kx][rci.jx+i][0], 1, &W[rci.ky][rci.jy+i][0], 1)
          ));
       if ( s > 0 ) {
          cblas_dscal(n, 1/s, &W[rci.kx][rci.jx+i][0], 1);
          cblas_dscal(n, 1/s, &W[rci.ky][rci.jy+i][0], 1);
       } else {
          for(int j=0; j<n; j++)
             W[rci.ky][rci.jy+i][j] = 0.0;
       }
    }
 }

amontoison · 2023-10-05T11:10:30Z

@amontoison also you will have to add CBLAS support for Meson, some of the SSMFE C examples require CBLAS functions for various matrix modification routines, eg:

 for(int i=0; i<rci.nx; i++) {
    if( rci.kx == rci.ky ) {
       double s = cblas_dnrm2(n, &W[rci.kx][rci.jx+i][0], 1);
       if( s > 0 )
          cblas_dscal(n, 1/s, &W[rci.kx][rci.jx+i][0], 1);
    } else {
       double s = sqrt(fabs(cblas_ddot(
          n, &W[rci.kx][rci.jx+i][0], 1, &W[rci.ky][rci.jy+i][0], 1)
          ));
       if ( s > 0 ) {
          cblas_dscal(n, 1/s, &W[rci.kx][rci.jx+i][0], 1);
          cblas_dscal(n, 1/s, &W[rci.ky][rci.jy+i][0], 1);
       } else {
          for(int j=0; j<n; j++)
             W[rci.ky][rci.jy+i][j] = 0.0;
       }
    }
 }

Do you mean a specific library for cblas?
We already support an option for blas and lapack.
We just expect a library with both symbols right now like openblas, mkl or apple accelerate.

jfowkes · 2023-10-05T11:52:49Z

Do you mean a specific library for cblas? We already support an option for blas and lapack. We just expect a library with both symbols right now like openblas, mkl or apple accelerate.

Sorry I meant to ensure it detects the CBLAS header cblas.h, this comes standard as part of all BLAS/LAPACK installations (it's just a C interface to the F77 BLAS routines).

amontoison · 2023-10-05T17:09:31Z

Oh, I throught that SIZEOF_IDX_T was a variable of metis.h but it's not the case.
I propose to replace the line https://github.com/ralna/spral/blob/master/src/metis5_wrapper.F90#L23 with #if IDXTYPEWIDTH == 64 directly. IDXTYPEWIDTH is defined by the metis header.

Yeah sure, that seems sensible to me. Why wasn't this done in the first place?

I don't know, it seems obvious to check IDXTYPEWIDTH directly.

We should also add some tests where we test against 64bit integer METIS so we know it works?

I added some tests with 64 bits integer METIS and it's not working.
I exactly have the same errors with FreeBSD, so I checked the metis.h file on FreeBSD and it's indeed a version with 64 bits integer.

amontoison · 2023-10-05T17:46:35Z

Do you mean a specific library for cblas? We already support an option for blas and lapack. We just expect a library with both symbols right now like openblas, mkl or apple accelerate.

Sorry I meant to ensure it detects the CBLAS header cblas.h, this comes standard as part of all BLAS/LAPACK installations (it's just a C interface to the F77 BLAS routines).

@jfowkes
I did a modification to ensure that the CLAS header cblas.h is correctly detected before that we run the tests and the examples of ssfme.

amontoison · 2023-10-06T06:56:26Z

@amontoison I think the issue is between the screen and the chair: we need to #include <metis.h> in metis5_wrapper.F90 otherwise the wrapper cannot see IDXTYPEWIDTH since it's defined in metis.h right?

Indeed, I pushed the fix.
If we ditch the macro SPRAL_HAVE_METIS_H, the compilation will fail if the header metis.h is not found.
Is it what you want?

jfowkes · 2023-10-06T07:35:58Z

Indeed, I pushed the fix. If we ditch the macro SPRAL_HAVE_METIS_H, the compilation will fail if the header metis.h is not found. Is it what you want?

Okay no let's not do that...

amontoison · 2023-10-06T22:52:31Z

@jfowkes
We can't include the header file "metis.h" in Fortran code.
We can only use it in C or C++ files.
I added an option metis64 (boolean) in Meson to specify if METIS was compiled with 64 bits integers.
We can update it once the old build system is removed.

jfowkes · 2023-10-07T10:14:51Z

@amontoison ah of course 🤦

Many thanks for your work on this! I guess this is ready for review then?

amontoison · 2023-10-07T12:25:22Z

Yes, It's ready now.

jfowkes · 2023-10-09T06:38:23Z

Great I'll add some documentation and merge it in!

jfowkes

🚀 🚀 🚀

amontoison added this to the Second Major Release milestone Sep 21, 2023

amontoison added the enhancement label Sep 21, 2023

amontoison self-assigned this Sep 21, 2023

This was referenced Sep 21, 2023

Add a meson build system #132

Closed

METIS 5 with 64bit integers only supported if --with-metis-inc-dir passed to configure #136

Open

jfowkes mentioned this pull request Sep 28, 2023

Possible bug in the code #144

Closed

amontoison force-pushed the meson branch from 880e010 to 21b1170 Compare October 1, 2023 19:36

amontoison force-pushed the meson branch 3 times, most recently from 17a1446 to b267b7b Compare October 2, 2023 07:14

amontoison commented Oct 3, 2023

View reviewed changes

meson.build Outdated Show resolved Hide resolved

amontoison force-pushed the meson branch from 2962515 to ca65ed8 Compare October 4, 2023 19:42

amontoison force-pushed the meson branch from dd0b6ff to bcfa276 Compare October 5, 2023 17:18

amontoison added 17 commits October 9, 2023 13:22

Add a meson build system

7873d8f

Support 64 bits integer in metis5_wrapper.F90

cda1fa2

Test if we have cblas.h

600e742

Test the SPRAL when METIS is compiled with 64 bits integer

e2907a9

Test the SPRAL when METIS is compiled with 64 bits integer

1842fee

Compile spral_ssids with SPRAL dependencies

4df0107

Update meson.yml

bb50b36

Update the name of the artifacts

503fc35

Check if the header file cblas.h is found

784ec06

Add the correct path for cblas.h

560369c

Update libspral_include

863d81e

Fix the flags generated by Meson

3ba92a3

Fix the issue between the chair and the screen

308317f

Update metis5_wrapper.F90

41508a4

Fix src/hw_topology/guess_topology.cxx

9489e67

Add an option metis64

de2877e

Add an option metis64

4041d38

jfowkes force-pushed the meson branch from b5a32e5 to 4041d38 Compare October 9, 2023 12:22

Add documentation for meson

20580f0

jfowkes self-requested a review October 9, 2023 13:51

jfowkes approved these changes Oct 9, 2023

View reviewed changes

jfowkes merged commit b02fd48 into master Oct 9, 2023
14 of 16 checks passed

jfowkes deleted the meson branch October 9, 2023 13:53

traversaro mentioned this pull request Oct 9, 2023

Enable CUDA support conda-forge/spral-feedstock#2

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a meson build system #141

Add a meson build system #141

amontoison commented Sep 21, 2023 •

edited

Loading

jfowkes commented Sep 21, 2023 •

edited

Loading

amontoison commented Sep 21, 2023 •

edited

Loading

amontoison commented Oct 1, 2023 •

edited

Loading

amontoison commented Oct 2, 2023 •

edited by jfowkes

Loading

jfowkes commented Oct 2, 2023

jfowkes commented Oct 3, 2023

amontoison commented Oct 3, 2023

jfowkes commented Oct 3, 2023

amontoison commented Oct 4, 2023 •

edited

Loading

jfowkes commented Oct 5, 2023 •

edited

Loading

amontoison commented Oct 5, 2023 •

edited

Loading

jfowkes commented Oct 5, 2023

jfowkes commented Oct 5, 2023 •

edited

Loading

amontoison commented Oct 5, 2023

jfowkes commented Oct 5, 2023

amontoison commented Oct 5, 2023 •

edited

Loading

amontoison commented Oct 5, 2023

amontoison commented Oct 6, 2023

jfowkes commented Oct 6, 2023 •

edited

Loading

amontoison commented Oct 6, 2023

jfowkes commented Oct 7, 2023

amontoison commented Oct 7, 2023 •

edited

Loading

jfowkes commented Oct 9, 2023

jfowkes left a comment

Add a meson build system #141

Add a meson build system #141

Conversation

amontoison commented Sep 21, 2023 • edited Loading

jfowkes commented Sep 21, 2023 • edited Loading

amontoison commented Sep 21, 2023 • edited Loading

amontoison commented Oct 1, 2023 • edited Loading

amontoison commented Oct 2, 2023 • edited by jfowkes Loading

jfowkes commented Oct 2, 2023

jfowkes commented Oct 3, 2023

amontoison commented Oct 3, 2023

jfowkes commented Oct 3, 2023

amontoison commented Oct 4, 2023 • edited Loading

jfowkes commented Oct 5, 2023 • edited Loading

amontoison commented Oct 5, 2023 • edited Loading

jfowkes commented Oct 5, 2023

jfowkes commented Oct 5, 2023 • edited Loading

amontoison commented Oct 5, 2023

jfowkes commented Oct 5, 2023

amontoison commented Oct 5, 2023 • edited Loading

amontoison commented Oct 5, 2023

amontoison commented Oct 6, 2023

jfowkes commented Oct 6, 2023 • edited Loading

amontoison commented Oct 6, 2023

jfowkes commented Oct 7, 2023

amontoison commented Oct 7, 2023 • edited Loading

jfowkes commented Oct 9, 2023

jfowkes left a comment

Choose a reason for hiding this comment

amontoison commented Sep 21, 2023 •

edited

Loading

jfowkes commented Sep 21, 2023 •

edited

Loading

amontoison commented Sep 21, 2023 •

edited

Loading

amontoison commented Oct 1, 2023 •

edited

Loading

amontoison commented Oct 2, 2023 •

edited by jfowkes

Loading

amontoison commented Oct 4, 2023 •

edited

Loading

jfowkes commented Oct 5, 2023 •

edited

Loading

amontoison commented Oct 5, 2023 •

edited

Loading

jfowkes commented Oct 5, 2023 •

edited

Loading

amontoison commented Oct 5, 2023 •

edited

Loading

jfowkes commented Oct 6, 2023 •

edited

Loading

amontoison commented Oct 7, 2023 •

edited

Loading