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

Weird type conversions or naming in Faiss #3628

Open
mdouze opened this issue Jul 11, 2024 · 5 comments
Open

Weird type conversions or naming in Faiss #3628

mdouze opened this issue Jul 11, 2024 · 5 comments

Comments

@mdouze
Copy link
Contributor

mdouze commented Jul 11, 2024

Summary

Platform

OS: Linux 64 intel

Faiss version: 1.8.0

Installed from: conda

Running on: CPU
Interface: Python

Reproduction instructions

The type mapping between c++ and Python integer represetnations is weitrd.

The script ttypes.py gives

(faiss_1.8.0) matthijs@devfair0459:~/src/NeuralCompressionInternal/faiss_index$ python ttypes.py 
int8 -> <Swig Object of type 'char *' at 0x7fce5d7c0a50>
uint8 -> <Swig Object of type 'uint8_t *' at 0x7fce5d7c0a20>
int16 -> <Swig Object of type 'int16_t *' at 0x7fce5d7c0a80>
uint16 -> <Swig Object of type 'uint16_t *' at 0x7fce5d7c0a20>
int32 -> <Swig Object of type 'faiss::HNSW::storage_idx_t *' at 0x7fce5d7c0a50>
uint32 -> <Swig Object of type 'unsigned int *' at 0x7fce5d7c0a20>
int64 -> <Swig Object of type 'int_fast16_t *' at 0x7fce5d7c0a80>
uint64 -> <Swig Object of type 'uintmax_t *' at 0x7fce5d7c0a20>
  • the int32 one is a weird naming, but is coherent
  • the int64 is probably wrong
  • the uint64 one is weird but probably right,

It is as if SWIG assigned a random name among all typedefs for a given type.

@junjieqi
Copy link
Contributor

@mdouze would you like to provide some more context? For example, what does prompt this question?

@mdouze
Copy link
Contributor Author

mdouze commented Jul 25, 2024

The context in which this problematic is when you want to implement a custom SWIG wrapper for a small C++ object as described in

https://github.com/facebookresearch/faiss/wiki/Python-C---code-snippets#wrapping-small-c-objects-for-use-from-python

I tried to do it in the following code

https://github.com/fairinternal/NeuralCompressionInternal/blob/dev/faiss_index/graph_search_traced.swig

but there is no way to define uint64_t is a way that is compatible with the main Faiss' implementation (so I resorted to passing pointers via void*)

@mdouze
Copy link
Contributor Author

mdouze commented Jul 25, 2024

TODO is (1) fix it and (2) add a test with a small C++ wrapper that actually tests this so that we catch regressions -- and this is also quite platform dependent as uint64_t is defined differently on platforms like windows.

@mdouze
Copy link
Contributor Author

mdouze commented Jul 29, 2024

Started working on this in #3699

@mdouze
Copy link
Contributor Author

mdouze commented Aug 9, 2024

Another datapoint: on the mac (conda 1.8.0), ttypes.py gives:

int8 -> <Swig Object of type 'char *' at 0x11f868480>
uint8 -> <Swig Object of type 'uint8_t *' at 0x11f868450>
int16 -> <Swig Object of type 'int16_t *' at 0x11f8684b0>
uint16 -> <Swig Object of type 'uint16_t *' at 0x11f868450>
int32 -> <Swig Object of type 'int_fast16_t *' at 0x11f868480>
uint32 -> <Swig Object of type 'uint_fast16_t *' at 0x11f868450>
int64 -> <Swig Object of type 'faiss::CMax< float,int64_t >::TI *' at 0x11f8684b0>
uint64 -> <Swig Object of type 'uintmax_t *' at 0x11f868450>

so wrong for int32, uint32

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

No branches or pull requests

4 participants