Fix SWIG 4.4 multi-phase init: replace import_array() with import_array1(-1)#4846
Closed
kyamagu wants to merge 2 commits into
Closed
Fix SWIG 4.4 multi-phase init: replace import_array() with import_array1(-1)#4846kyamagu wants to merge 2 commits into
kyamagu wants to merge 2 commits into
Conversation
SWIG 4.4+ generates SWIG_mod_exec() returning int (PEP 489 multi-phase init), while older SWIG generates PyInit_() returning PyObject*. NumPy's import_array() expands to 'return NULL' and import_array1(-1) expands to 'return -1', each incompatible with one of the two return types. Fix by isolating the NumPy import into a helper returning int, then returning the appropriate error value from %init based on SWIG_VERSION. Fixes: #4845
kyamagu
force-pushed
the
fix/swig44-import-array
branch
from
2c00f54 to
5c3c0bf
Compare
Contributor
|
@mnorris11 has imported this pull request. If you are a Meta employee, you can view this in D94921302. |
Contributor
|
Hi @kyamagu thanks for the report and contribution, my repro is WIP. If the import_array1 fails and returns |
Contributor
Author
|
@mnorris11 import_array() is a macro that returns on failure, and the |
Contributor
Got it thanks! I was not able to repro in https://github.com/facebookresearch/faiss/actions/runs/22603509873/job/65492234884?pr=4853 (all I did was change swig version to 4.4 in the yml, tests still pass) but the change itself seems fine. Is this something you saw with pip installs? |
Contributor
Author
|
@mnorris11 This was causing the error when building a wheel package on Linux arm64 arch + cpython 3.10 + numpy >= 2.0 + swig 4.4.1: https://github.com/faiss-wheels/faiss-wheels/actions/runs/22444196417/job/64994127319 I'm not sure what is the difference there, but one possibility is the numpy version? |
Contributor
|
@mnorris11 merged this pull request in 28f79bd. |
Contributor
Author
|
This error is likely specific to musllinux environment. On manylinux aarch64: On musllinux aarch64: |
dimitraseferiadi
pushed a commit
to dimitraseferiadi/SuCo
that referenced
this pull request
Mar 8, 2026
…ay1(-1) (facebookresearch#4846) Summary: ## Problem Building faiss with SWIG 4.4.0+ fails to compile the generated wrapper: ``` In function 'int SWIG_mod_exec(PyObject*)': error: cannot convert 'std::nullptr_t' to 'int' in return import_array(); ``` SWIG 4.4.0 introduced PEP 489 multi-phase module initialization, where `SWIG_mod_exec` returns `int` instead of `PyObject*`. NumPy's `import_array()` macro expands to `return NULL`, which is a type mismatch. Fixes facebookresearch#4845. ## Why not just use `import_array1(-1)`? `import_array1(-1)` expands to `return -1`. This fixes SWIG 4.4+ but breaks older SWIG versions, which generate `PyInit_()` returning `PyObject*` — and the upstream CI (SWIG 4.0.2) confirmed exactly this failure: ``` In function 'PyObject* PyInit__swigfaiss()': error: invalid conversion from 'int' to 'PyObject*' import_array1(-1); ``` ## Fix Isolate the NumPy import into a helper function that always returns `int`, then return the correct error value from `%init` using a `SWIG_VERSION` preprocessor check: ```diff +%{ +static int _faiss_init_numpy() { + import_array1(-1); + return 0; +} +%} + %init %{ - import_array(); + if (_faiss_init_numpy() < 0) { +#if SWIG_VERSION >= 0x040400 + return -1; +#else + return NULL; +#endif + } PythonInterruptCallback::reset(); %} ``` - `_faiss_init_numpy()` always returns `int`, so `import_array1(-1)` inside it compiles cleanly under any SWIG version - The `%init` block then returns the correct type for its enclosing function: `-1` (int) for SWIG 4.4+ multi-phase init, `NULL` (PyObject*) for older single-phase init Pull Request resolved: facebookresearch#4846 Reviewed By: junjieqi Differential Revision: D94921302 Pulled By: mnorris11 fbshipit-source-id: 8d4fab61ffce76a10ee7b3f00bab87471b48e02a
dimitraseferiadi
pushed a commit
to dimitraseferiadi/SuCo
that referenced
this pull request
Mar 16, 2026
…ay1(-1) (facebookresearch#4846) Summary: ## Problem Building faiss with SWIG 4.4.0+ fails to compile the generated wrapper: ``` In function 'int SWIG_mod_exec(PyObject*)': error: cannot convert 'std::nullptr_t' to 'int' in return import_array(); ``` SWIG 4.4.0 introduced PEP 489 multi-phase module initialization, where `SWIG_mod_exec` returns `int` instead of `PyObject*`. NumPy's `import_array()` macro expands to `return NULL`, which is a type mismatch. Fixes facebookresearch#4845. ## Why not just use `import_array1(-1)`? `import_array1(-1)` expands to `return -1`. This fixes SWIG 4.4+ but breaks older SWIG versions, which generate `PyInit_()` returning `PyObject*` — and the upstream CI (SWIG 4.0.2) confirmed exactly this failure: ``` In function 'PyObject* PyInit__swigfaiss()': error: invalid conversion from 'int' to 'PyObject*' import_array1(-1); ``` ## Fix Isolate the NumPy import into a helper function that always returns `int`, then return the correct error value from `%init` using a `SWIG_VERSION` preprocessor check: ```diff +%{ +static int _faiss_init_numpy() { + import_array1(-1); + return 0; +} +%} + %init %{ - import_array(); + if (_faiss_init_numpy() < 0) { +#if SWIG_VERSION >= 0x040400 + return -1; +#else + return NULL; +#endif + } PythonInterruptCallback::reset(); %} ``` - `_faiss_init_numpy()` always returns `int`, so `import_array1(-1)` inside it compiles cleanly under any SWIG version - The `%init` block then returns the correct type for its enclosing function: `-1` (int) for SWIG 4.4+ multi-phase init, `NULL` (PyObject*) for older single-phase init Pull Request resolved: facebookresearch#4846 Reviewed By: junjieqi Differential Revision: D94921302 Pulled By: mnorris11 fbshipit-source-id: 8d4fab61ffce76a10ee7b3f00bab87471b48e02a
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Building faiss with SWIG 4.4.0+ fails to compile the generated wrapper:
SWIG 4.4.0 introduced PEP 489 multi-phase module initialization, where
SWIG_mod_execreturnsintinstead ofPyObject*. NumPy'simport_array()macro expands toreturn NULL, which is a type mismatch.Fixes #4845.
Why not just use
import_array1(-1)?import_array1(-1)expands toreturn -1. This fixes SWIG 4.4+ but breaks older SWIG versions, which generatePyInit_()returningPyObject*— and the upstream CI (SWIG 4.0.2) confirmed exactly this failure:Fix
Isolate the NumPy import into a helper function that always returns
int, then return the correct error value from%initusing aSWIG_VERSIONpreprocessor check:_faiss_init_numpy()always returnsint, soimport_array1(-1)inside it compiles cleanly under any SWIG version%initblock then returns the correct type for its enclosing function:-1(int) for SWIG 4.4+ multi-phase init,NULL(PyObject*) for older single-phase init