Add support for IndexIDMap with Cagra fp16

[jinsolp]

This PR does 2 things

• Enable support fot IndexIDMap with Cagra fp16 (original support introduced in Add the support for IndexIDMap with Cagra index #4188)
  • Added tests in test_cagra.py
• Reflecting feedback about python API from cuVS Cagra FP16 support #4384 (comment)

[jinsolp]

One of the issues that I've noticed is that the current extended API approach results in a lot of warnings from all child structs at compile time. This is because we have an overload of a function (e.g. add(n, x) and add(n, x, faiss.Float16)) in the base struct (Index.h), but the child structs only override one of these overloaded functions (because most of them don't bother about the latter add).

The straightforward solution is to add using Index::add; to every single child struct, but this seems very tedious... (and we will have to do it for every extended-api function)
Any suggestions to tackling this??

[mdouze]

For this change to work without user visible degradation, we should implement implicit conversion fp16->fp32 in C++ for add/search/train. There are a few fp16 routines for CPU here: https://github.com/facebookresearch/faiss/blob/main/faiss/utils/fp16.h

[mdouze]

makes me think that the codepath to provide fp16 or bf16 pytorch arrays is here:

faiss/contrib/torch_utils.py

Line 139 in bb58f6d

assert type(x) is torch.Tensor

these have not been adapted to the type-tagged add/train/search variants.

[jinsolp]

@mdouze Thanks for your feedback! I have made a few changes : )

• Renamed all extended APIs with the Ex postfix because of cpp compile warnings related to not overriding overloaded functions.
• Added the numeric_type tag support for torch_utils.py

[mdouze]

I don't understand the rationale for the Ex suffix. Where does the cpp compiler issue warnings?

[jinsolp]

@mdouze In the current CI runs for cuVS build, it can be seen that it raises a warning for every single index https://github.com/facebookresearch/faiss/actions/runs/16378195325/job/46284306200

e.g. overloaded virtual function "faiss::Index::train" is only partially overridden in class "faiss::IndexPQ"

I've stated the reason for this in the comment above

One of the issues that I've noticed is that the current extended API approach results in a lot of warnings from all child structs at compile time. This is because we have an overload of a function (e.g. add(n, x) and add(n, x, faiss.Float16)) in the base struct (Index.h), but the child structs only override one of these overloaded functions (because most of them don't bother about the latter add).
The straightforward solution is to add using Index::add; to every single child struct, but this seems very tedious... (and we will have to do it for every extended-api function)

TLDR; warning happen because most child structs don't override the overloaded functions (i.e. most child structs don't override the extended API).

There are two ways to tackle this cpp warning

1. add using in every single index struct so that it can see the base Index struct's overloaded functions with numeric types (the extended functions)

• pro: we have the same name: the original add and the add with void* and numeric_type.
• con: we have to change every single index in FAISS

2. change the name of the extended functions so that they are not an "overload". i.e. instead of having two add functions with different signatures, just have add and addEx

• pro: we only need to change a few files where we are using the extended API
• con: we have the *Ex postfix for extended API

[mnorris11]

Sorry for delayed responses. At least to me, it does not seem like a problem to annotate this 1 line on all the indexes that require it, if the API stays as add(), etc. It looks cleaner and keeps with the current pattern.

[jinsolp]

Got it, thanks @mnorris11 , I'll fix this today and push : )

[facebook-github-bot]

@mnorris11 has imported this pull request. If you are a Meta employee, you can view this in D78695771.

[jinsolp]

@mnorris11 Now the diff is really big, but we are 1) keeping the previous API (without the Ex) and 2) compiling without warnings.

swig wasn't able to process macros or work with using. So what I've done is manually override functions if the child struct is already overriding its original counterpart. (i.e. if the child class is already overriding add() I also overrode add(void*, numeric_type).
All overrode functions are just forwarded to its base struct's function to keep the logic consistent as before.

[facebook-github-bot]

@mnorris11 has imported this pull request. If you are a Meta employee, you can view this in D78695771.

[facebook-github-bot]

@mnorris11 merged this pull request in 71f775b.