DrTimothyAldenDavis / SuiteSparse

The official SuiteSparse library: a suite of sparse matrix algorithms authored or co-authored by Tim Davis, Texas A&M University.
https://people.engr.tamu.edu/davis/suitesparse.html
Other
1.15k stars 259 forks source link

Build failed when compiling to mexfile #872

Open smsimone opened 1 week ago

smsimone commented 1 week ago

Since I was on Ubuntu 22.04 and libsuitesparse-dev:5.10.1 my application did compile correctly, now I've updated my system to ubuntu 24.04 with libsuitesparse-dev:7.6.1 and it does not compile anymore with the error

error: #error Using MATLAB Data API with C Matrix API is not supported.

I've tried to use the mex compiler from both Matlab 23.2 and Matlab 24.1.

Between the logs I've found also this statement here (it was after the matlab error) which I don't know if it belongs to the previous error or not:

conflicting declaration of C function ‘void mexFunction(int, mxArray**, int, const mxArray**)’
  356 | void mexFunction(
      |      ^~~~~~~~~~~
mmuetzel commented 1 week ago

Could you please share the complete context of the first error message?

smsimone commented 1 week ago

Sure, bere it is

The first one


In file included from /home/gioana/Programmi/MATLAB/extern/include/mex.h:43,
                 from /usr/include/suitesparse/SuiteSparse_config.h:88,
                 from /usr/include/suitesparse/cholmod.h:308,
                 from /home/gioana/Scrivania/Libreria_VEM/roots_3D1D/addim/gedim/GeDiM/src/Algebra/SuiteSparse_Utilities.hpp:12,
                 from /home/gioana/Scrivania/Libreria_VEM/roots_3D1D/addim/gedim/GeDiM/src/Algebra/Eigen_SparseArray.hpp:10,
                 from /home/gioana/Scrivania/Libreria_VEM/roots_3D1D/Roots/Roots_Natural_VEM_3D_1D/src/Problem/CGSparseMatrixFree.hpp:4,
                 from /home/gioana/Scrivania/Libreria_VEM/roots_3D1D/Roots/Roots_Natural_VEM_3D_1D/src/Problem/RootsApplication_CGSparseMatrixFree.hpp:7,
                 from /home/gioana/Scrivania/Libreria_VEM/roots_3D1D/Roots/Roots_Natural_VEM_3D_1D/src/Problem/RootsProblemConfiguration.hpp:8,
                 from /home/gioana/Scrivania/Libreria_VEM/roots_3D1D/Roots/Roots_Natural_VEM_3D_1D/src/RegisterDependencies.hpp:7,
                 from /home/gioana/Scrivania/Libreria_VEM/roots_3D1D/Roots/Roots_Natural_VEM_3D_1D/mexfunction.cpp:15:
/home/gioana/Programmi/MATLAB/extern/include/matrix.h:11:2: error: #error Using MATLAB Data API with C Matrix API is not supported.
   11 | #error Using MATLAB Data API with C Matrix API is not supported.
mmuetzel commented 1 week ago

IIUC, this error is emitted if you include MDArray.hpp before including mex.h.

I'm not entirely sure why SuiteSparse_config.h automatically includes mex.h when building mex files. Maybe, @DrTimothyAldenDavis could give some background. But afaict, that makes it difficult to use SuiteSparse headers in files using the C++ mex API (which should include mex.hpp instead): https://www.mathworks.com/help/matlab/cpp-mex-file-applications.html

@DrTimothyAldenDavis: Wouldn't it be cleaner if it were left to the user to include Matlab headers as needed? Or use a SuiteSparse-specific macro if they intent to use the SuiteSparse_config_struct (and related) facilities (which only work in C code anyway). Would it be ok to basically replace MATLAB_MEX_FILE with SUITESPARSE_MATLAB_MEX_FILE everywhere in SuiteSparse? That would mean that you would need to add #define SUITESPARSE_MATLAB_MEX_FILE in your .mex file sources before including SuiteSparse headers. But it would allow others to include SuiteSparse headers in sources using the C++ mex API (by not defining that).

@smsimone: Does it make a difference if you include the SuiteSparse headers before mex.hpp? (That's not a solution. Just an attempt of a potential workaround.)

DrTimothyAldenDavis commented 1 week ago

When SuiteSparse_config is compiled for a mexFunction, it sets the default memory allocators to be mxMalloc and company; you don't want SuiteSparse to use malloc/free when inside a MATLAB mexFunction. That's why it needs mex.h. Both SuiteSparse_config.h and mex.h are C header files.

Can you try placing SuiteSparse_config.h inside an extern "C" { ... } block?

mmuetzel commented 1 week ago

Can you try placing SuiteSparse_config.h inside an extern "C" { ... } block?

That probably won't help. That error message in matrix.h is guarded like this:

#ifdef MDA_ARRAY_HPP_
#error Using MATLAB Data API with C Matrix API is not supported.
#endif

MDA_ARRAY_HPP_ is the inclusion guard that is defined when MDArray.hpp is included. That header is pulled in when a user adds #include "mex.hpp" in their sources.

So, it won't make a difference whether or not SuiteSparse_config.h is included in an extern "C" block...

mmuetzel commented 1 week ago

When SuiteSparse_config is compiled for a mexFunction, it sets the default memory allocators to be mxMalloc and company; you don't want SuiteSparse to use malloc/free when inside a MATLAB mexFunction. That's why it needs mex.h. Both SuiteSparse_config.h and mex.h are C header files.

That sounds like a pretty annoying limitation. If that really is necessary, that might mean that users that would like to use the C++ mex API and SuiteSparse would need to be careful to keep the two in separate compilation units. Linking these compilation units to a single .mex file should still be possible though...

DrTimothyAldenDavis commented 1 week ago

OK ... I'll think about a workaround for the current version (a hack to /usr/include/suitesparse/SuiteSparse_config.h for @smsimone so the existing libsuitesparse-dev:7.6.1 can be used), and then also a cleaner fix for SuiteSparse 7.8.3.

The hack to SuiteSparse_config.h might be something like change its "#include 'mex.h'" to

#ifdef __cplusplus
#include "mex.hpp"
#else 
#include "mex.h"
#endif

Alternatively, the inclusion of mex.h could be avoided in the end user application by adding

#define mex_h
#define matrix_h

before including SuiteSparse_config.h. That would allow the existing SuiteSparse_config.h from v7.6.1 to work without any changes to that file.

DrTimothyAldenDavis commented 1 week ago

I revised the draft SuiteSparse 7.8.3 to fix this issue: https://github.com/DrTimothyAldenDavis/SuiteSparse/pull/868/commits/39c23ae9a82b1668c61285691bc679bf61a1b796

The mex.h and matrix.h files are no longer #include'd in SuiteSparse_config.h in that update.

To workaround your issue and continue to use the libsuitesparse-dev:7.6.1, you could either edit your /usr/include/suitesparse/SuiteSparse_config.h file to remove those 2 lines, or you can try adding these lines to your own application, before including any SuiteSparse packages, if you can't edit your /usr/include/suitesparse/SuiteSparse_config.h file:

#define mex_h
#define matrix_h

Let me know if either works.