Faiss: v1.6.2 : Compilation of C API Failed

Created on 12 Mar 2020  Â·  7Comments  Â·  Source: facebookresearch/faiss

Summary

the C api compilation failed on Clustering_c.cpp compilation

Faiss version: v1.6.2

Reproduction instructions

FROM ubuntu:18.04 as builder

ARG FAISS_HOST=https://github.com/facebookresearch/faiss
ARG FAISS_VERSION=1.6.2

RUN apt-get update && apt-get -y install gcc build-essential libblas-dev liblapack-dev
RUN apt-get install -y wget

RUN wget -nv -O faiss.tar.gz ${FAISS_HOST}/archive/v${FAISS_VERSION}.tar.gz
RUN tar -zxf faiss.tar.gz
WORKDIR /faiss-${FAISS_VERSION}
RUN ./configure --without-cuda && make
WORKDIR /faiss-${FAISS_VERSION}/c_api
RUN make
g++ -std=c++11 -DFINTEGER=int  -fopenmp  -fPIC -m64 -Wno-sign-compare -g -O3 -Wall -Wextra -I.. -I ../impl -DNDEBUG  -mpopcnt -msse4 -c error_impl.cpp -o error_impl.o
g++ -std=c++11 -DFINTEGER=int  -fopenmp  -fPIC -m64 -Wno-sign-compare -g -O3 -Wall -Wextra -I.. -I ../impl -DNDEBUG  -mpopcnt -msse4 -c Index_c.cpp -o Index_c.o
g++ -std=c++11 -DFINTEGER=int  -fopenmp  -fPIC -m64 -Wno-sign-compare -g -O3 -Wall -Wextra -I.. -I ../impl -DNDEBUG  -mpopcnt -msse4 -c IndexFlat_c.cpp -o IndexFlat_c.o
g++ -std=c++11 -DFINTEGER=int  -fopenmp  -fPIC -m64 -Wno-sign-compare -g -O3 -Wall -Wextra -I.. -I ../impl -DNDEBUG  -mpopcnt -msse4 -c Clustering_c.cpp -o Clustering_c.o
Clustering_c.cpp: In function 'void faiss_Clustering_obj(FaissClustering*, float**, size_t*)':
Clustering_c.cpp:85:72: error: 'struct faiss::Clustering' has no member named 'obj'
     std::vector<float>& v = reinterpret_cast<Clustering*>(clustering)->obj;
                                                                        ^~~
make: *** [Clustering_c.o] Error 1
Makefile:42: recipe for target 'Clustering_c.o' failed
enhancement

All 7 comments

It appears that Clustering has been recent changed to provide a sequence of iteration stats instead of just the objective values (Clustering.h:76).

Would you like to try and correct this? It should amount to:

  1. Creating the corresponding C version of ClusteringIterationStats, with enough getters to the inner fields;
  2. Replacing the function faiss_Clustering_obj with a new faiss_Clustering_iteration_stats.

Feel free to ask follow-up questions if in doubt.

ok i'll try

do you know this project follow any policy or rules about backward compatibility and versioning ?

Can't speak for the maintainers, but my policy on the C API has been that it should try to be as compatible as possible with the public upstream code. Whenever the native API changes, the C API may follow suit. Bindings that consume this API may then need to explicitly state which version of the library they are relying on, without assuming semantic versioning.

I tried to run the c api compilation in CI
https://github.com/glutamatt/faiss/commit/544d075d4306673ac5ff52f4f2beb25605cf4a22

it works well
pass after my fix : https://travis-ci.org/github/glutamatt/faiss/jobs/661617512#L1732
fail if runned in CI before my fix : https://travis-ci.org/github/glutamatt/faiss/jobs/661623278#L1743

would it make sense to add this c api make step in CI ?

Building the C API in CI could become a hard blocker for development iterations, but a good trade-off might be to add it with allowed failures, so that it can be tracked independently.

Ok I got it
I'll submit a pull request based on your advice

I'll accept the PR if @Enet4 agrees with it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Tony-Hou picture Tony-Hou  Â·  3Comments

xxllp picture xxllp  Â·  3Comments

hashyong picture hashyong  Â·  3Comments

daniellevy picture daniellevy  Â·  3Comments

mylyu picture mylyu  Â·  3Comments