Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build warnings in the Windows Meson build we should fix #120

Closed
Tracked by #36
rgommers opened this issue Jan 18, 2022 · 6 comments
Closed
Tracked by #36

Build warnings in the Windows Meson build we should fix #120

rgommers opened this issue Jan 18, 2022 · 6 comments

Comments

@rgommers
Copy link
Owner

@Smit-create here are all the relevant build warnings from today's Windows build against numpy 1.21.5. Could you look into these?

[325/1557] Compiling C object scipy/sparse/linalg/_dsolve/libsuperlu_lib.a.p/SuperLU_SRC_cmemory.c.obj
../scipy/sparse/linalg/_dsolve/SuperLU/SRC/cmemory.c: In function 'cStackCompress':
../scipy/sparse/linalg/_dsolve/SuperLU/SRC/cmemory.c:649:24: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  649 |     Glu->stack.used -= (long int) fragment;
      |                        ^
../scipy/sparse/linalg/_dsolve/SuperLU/SRC/cmemory.c:650:24: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
  650 |     Glu->stack.top1 -= (long int) fragment;
      |                        ^

The above one also happens in dStackCompress and the s/z variants.

[357/1557] Compiling C++ object scipy/special/_ufuncs_cxx.cp39-win_amd64.pyd.p/meson-generated__ufuncs_cxx.cpp.obj
scipy/special/_ufuncs_cxx.cp39-win_amd64.pyd.p/_ufuncs_cxx.cpp:659: warning: "_USE_MATH_DEFINES" redefined
  659 |   #define _USE_MATH_DEFINES
      | 
<command-line>: note: this is the location of the previous definition
In file included from C:\hostedtoolcache\windows\Python\3.9.9\x64\lib\site-packages\numpy\core\include/numpy/npy_math.h:8,
                 from ..\scipy\special/_complexstuff.h:5,
                 from scipy/special/_ufuncs_cxx.cp39-win_amd64.pyd.p/_ufuncs_cxx.cpp:694:
[516/1557] Compiling C object scipy/stats/_unuran/unuran_wrapper.cp39-win_amd64.pyd.p/.._..__lib_unuran_unuran_src_methods_mvtdr.c.obj
In file included from ../scipy/_lib/unuran/unuran/src/methods/mvtdr.c:330:
In function '_unur_mvtdr_etable_new',
    inlined from '_unur_mvtdr_triangulate' at ../scipy/_lib/unuran/unuran/src/methods/mvtdr_init.h:978:11:
../scipy/_lib/unuran/unuran/src/methods/mvtdr_init.h:1760:17: warning: argument 1 value '18446744073709551608' exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
 1760 |   GEN->etable = malloc( size * sizeof(E_TABLE*) );
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ..\scipy\_lib\unuran\unuran\src/unur_source.h:71,
                 from ../scipy/_lib/unuran/unuran/src/methods/mvtdr.c:50:
../scipy/_lib/unuran/unuran/src/methods/mvtdr_init.h: In function '_unur_mvtdr_triangulate':
c:/rtools40/ucrt64/x86_64-w64-mingw32/include/stdlib.h:531:17: note: in a call to allocation function 'malloc' declared here
  531 |   void *__cdecl malloc(size_t _Size);
      |                 ^~~~~~
[564/1557] Compiling C object scipy/special/cython_special.cp39-win_amd64.pyd.p/meson-generated_cython_special.c.obj
scipy/special/cython_special.cp39-win_amd64.pyd.p/cython_special.c:645: warning: "_USE_MATH_DEFINES" redefined
  645 |   #define _USE_MATH_DEFINES
      | 
<command-line>: note: this is the location of the previous definition
In file included from C:\hostedtoolcache\windows\Python\3.9.9\x64\lib\site-packages\numpy\core\include/numpy/ndarraytypes.h:4,
                 from C:\hostedtoolcache\windows\Python\3.9.9\x64\lib\site-packages\numpy\core\include/numpy/ndarrayobject.h:12,
                 from C:\hostedtoolcache\windows\Python\3.9.9\x64\lib\site-packages\numpy\core\include/numpy/arrayobject.h:4,
                 from scipy/special/cython_special.cp39-win_amd64.pyd.p/cython_special.c:681:
scipy/signal/_upfirdn_apply.cp39-win_amd64.pyd.p/_upfirdn_apply.c: In function '__pyx_fuse_1__pyx_pw_5scipy_6signal_14_upfirdn_apply_21_apply':
scipy/signal/_upfirdn_apply.cp39-win_amd64.pyd.p/_upfirdn_apply.c:14631:14: warning: '__pyx_v_row_size_bytes' may be used uninitialized in this function [-Wmaybe-uninitialized]
14631 |       (void)(memset(__pyx_v_output_row, 0, __pyx_v_row_size_bytes));
      |             ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scipy/signal/_upfirdn_apply.cp39-win_amd64.pyd.p/_upfirdn_apply.c:14056:10: note: '__pyx_v_row_size_bytes' was declared here
14056 |   size_t __pyx_v_row_size_bytes;
      |          ^~~~~~~~~~~~~~~~~~~~~~

Many warnings like:

[936/1557] Compiling Fortran object scipy/sparse/linalg/_eigen/arpack/libarpack_lib.a.p/ARPACK_SRC_sgetv0.f.obj
../scipy/sparse/linalg/_eigen/arpack/ARPACK/SRC/sgetv0.f:404:32:

  367 |           call svout (logfil, 1, rnorm0, ndigit,
      |                                 2
......
  404 |          call svout (logfil, n, resid, ndigit,
      |                                1
Warning: Rank mismatch between actual argument at (1) and actual argument at (2) (scalar and rank-1)

These rank mismatch warnings should be silenced by using -fallow-argument-mismatch, see the TODO comment in the top-level meson.build.

Also recording this one, but needs someone who speaks Fortran:

[999/1557] Compiling Fortran object scipy/stats/_mvn.cp39-win_amd64.pyd.p/mvndst.f.obj
../scipy/stats/mvndst.f:233:39:

  233 |       DOUBLE PRECISION COV(NL*(NL+1)/2), A(NL), B(NL), Y(NL)
      |                                       1
Warning: Array 'cov' at (1) is larger than limit set by '-fmax-stack-var-size=', moved from stack to static storage. This makes the procedure unsafe when called recursively, or concurrently from multiple threads. Consider using '-frecursive', or increase the '-fmax-stack-var-size=' limit, or change the code to use an ALLOCATABLE array. [-Wsurprising]

Not sure what the deal is with this one:

[1025/1557] Linking target scipy/stats/_biasedurn.cp39-win_amd64.pyd
Warning: corrupt .drectve at end of def file
Warning: corrupt .drectve at end of def file
Warning: corrupt .drectve at end of def file
../scipy/odr/__odrpack.c: In function 'odr':
../scipy/odr/__odrpack.c:1121:21: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'npy_intp' {aka 'long long int'} [-Wformat=]
 1121 |           printf("%ld %lld\n", PyArray_DIMS(work)[0], (long long)lwork);
      |                   ~~^          ~~~~~~~~~~~~~~~~~~~~~
      |                     |                            |
      |                     long int                     npy_intp {aka long long int}
      |                   %lld
../scipy/odr/__odrpack.c:1121:21: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'npy_intp' {aka 'long long int'} [-Wformat=]
 1121 |           printf("%ld %lld\n", PyArray_DIMS(work)[0], (long long)lwork);
      |                   ~~^          ~~~~~~~~~~~~~~~~~~~~~
      |                     |                            |
      |                     long int                     npy_intp {aka long long int}
      |                   %lld

Let's prioritize this one, since currently linprog (which uses HiGHS) is segfaulting:

[1474/1557] Compiling C++ object scipy/optimize/_highs/libhighs.a.p/.._..__lib_highs_src_mip_HighsImplications.cpp.obj
../scipy/_lib/highs/src/mip/HighsImplications.cpp: In member function 'bool HighsImplications::runProbing(HighsInt, HighsInt&)':
../scipy/_lib/highs/src/mip/HighsImplications.cpp:167:46: warning: 'implicsdown' may be used uninitialized in this function [-Wmaybe-uninitialized]
  167 |       if (implicsup[u].column < implicsdown[d].column)
      |  
@Smit-create
Copy link

Could you look into these?

Yeah, will look into it.

@Smit-create
Copy link

With reference to this: https://github.com/rgommers/scipy/runs/5123938429?check_suite_focus=true

  1. This is the warning from autogenerated code:
scipy/linalg/_interpolativemodule.c:554:1: warning: 'thread' attribute directive ignored [-Wattributes]
  554 | static F2PY_THREAD_LOCAL_DECL cb_matvect_in_idd__user__routines_t *_active_cb_matvect_in_idd__user__routines = NULL;
      | ^~~~~~

@rgommers
Copy link
Owner Author

Okay, let's leave that one alone for now then. I think it is fixed in a more recent NumPy version already. At least we fixed one thread-related warning in f2py recently.

@Smit-create
Copy link

1760 | GEN->etable = malloc( size * sizeof(E_TABLE*) );

This comes from the _unuran submodule. A probable fix for this is:

diff --git a/unuran/src/methods/mvtdr_init.h b/unuran/src/methods/mvtdr_init.h
index 333b693..42ca01e 100644
--- a/unuran/src/methods/mvtdr_init.h
+++ b/unuran/src/methods/mvtdr_init.h
@@ -1757,7 +1757,7 @@ _unur_mvtdr_etable_new( struct unur_gen *gen, int size )
   GEN->etable_size = size;
 
   /* make root */
-  GEN->etable = malloc( size * sizeof(E_TABLE*) );
+  GEN->etable = malloc(_unur_min(size * sizeof(E_TABLE*), 9223372036854775807));
   if (GEN->etable==NULL) {
     _unur_error(gen->genid,UNUR_ERR_MALLOC,""); return UNUR_ERR_MALLOC; }

So, do I need to fix that here https://github.com/scipy/unuran.git?

@rgommers
Copy link
Owner Author

@Smit-create yes indeed, that is the place to make a change. That fix doesn't look correct though (using a hardcoded number like that is almost always a sign of trouble). The problem here seems to be that in this line:

GEN->etable = malloc( size * sizeof(E_TABLE*) );

E_TABLE isn't defined. Searching further in that file:

VERTEX *
_unur_mvtdr_etable_find_or_insert( struct unur_gen *gen, VERTEX **vidx )
...
  E_TABLE *pet, *pet_last;  /* table entry */
  int idx[2];               /* store indices */
  int hidx;                 /* hash number */
...
pet = malloc( sizeof(E_TABLE) );

However, that's the wrong E_TABLE struct. There's a global definition in mvtdr_struct.h:

typedef struct s_edge_table       /* -- hash table for edges --------------- */
{
  int  index[2];                  /* index of incident vertices              */
  VERTEX *vertex;                 /* index of corresponding vertex (=barycenter) */
  struct s_edge_table *next;      /* next entry in list                      */
} E_TABLE;

The code seems to rely on both of these files being included in the correct order in mvtdr.c.

I'm guessing the fix is to use sizeof(E_TABLE) instead of sizeof(E_TABLE*). It's not clear to me though why this code works, and only gives a build warning on Windows.

Cc @tirthasheshpatel, in case this code is familiar to you.

@rgommers
Copy link
Owner Author

Looks like we've fixed what needs fixing for now (modulo some discussion on the interpolative warnings PR). There's some warnings left, TBD later whether they need silencing. I'll close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants