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

Update INSTALL.md #1456

Closed
wants to merge 3 commits into from
Closed

Update INSTALL.md #1456

wants to merge 3 commits into from

Conversation

mdouze
Copy link
Contributor

@mdouze mdouze commented Oct 12, 2020

Added doc + placeholders on how to compile demos and tests with cmake

Added doc + placeholders on how to compile demos and tests with cmake
INSTALL.md Outdated
@@ -310,6 +304,8 @@ libfaiss.so (or libfaiss.dylib)
the executable should be linked to one of these. If you use
the static version (.a), add the LDFLAGS used in the Makefile.

** is this up-to-date? Does cmake generate dynamic libs? **
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMake generate shared libs iff the -DBUILD_SHARED_LIBS=ON option is passed to the cmake invocation.

Comment on lines -121 to +114
which you can build by calling
`make demo_ivfpq_indexing`
which you can build by calling
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the command disappeared.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build: $ make -C build demo_ivfpq_indexing
run: $ ./build/demo_ivfpq_indexing

Comment on lines -121 to +114
which you can build by calling
`make demo_ivfpq_indexing`
which you can build by calling
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build: $ make -C build demo_ivfpq_indexing
run: $ ./build/demo_ivfpq_indexing

INSTALL.md Outdated

It makes a small index, stores it and performs some searches. A normal
runtime is around 20s. With a fast machine and Intel MKL's BLAS it
runs in 2.5s.

To run the whole test suite:

`make test` (for the CPU part)
** how to run the C++ tests **
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ make -C build test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does not work

(faiss_latest_cmake) matthijs@devfair0144:~/faiss_versions/faiss_latest_cmake$ make -C build test
make: Entering directory '/private/home/matthijs/faiss_versions/faiss_latest_cmake/build'
make: *** No rule to make target 'test'.  Stop.
make: Leaving directory '/private/home/matthijs/faiss_versions/faiss_latest_cmake/build'
(faiss_latest_cmake) matthijs@devfair0144:~/faiss_versions/faiss_latest_cmake$ make -C build tests
make: Entering directory '/private/home/matthijs/faiss_versions/faiss_latest_cmake/build'
make: *** No rule to make target 'tests'.  Stop.
make: Leaving directory '/private/home/matthijs/faiss_versions/faiss_latest_cmake/build'

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to build the tests, they need to be enabled via -DBUILD_TESTING=ON. The option should be ON by default, but it is currently not, I'll send a fix.

INSTALL.md Outdated
make demos
./demos/demo_sift1M
```
** how to compile demo_sift1M **
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ make -C build demo_sift1M

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ cmake -B build -DBLA_VENDOR=Intel10_64_dyn -DFAISS_ENABLE_GPU=OFF .
$ make -C build -j demo_sift1M
works fine for me. There may be an issue with your MKL installation?

Comment on lines +153 to +155
SWIG generates two wrapper files: a Python file `swigfaiss.py` and a
C++ file that must be compiled to a dynamic library (`_swigfaiss.so`).
There is an AVX2 variant of the files, suffixed with `_avx2`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is not really relevant to installing, and is probably more confusing than helpful to new users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will move it to the packaging section.

INSTALL.md Outdated

Testing the GPU implementation
------------------------------

Compile the example with

** update **
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ make -C build demo_ivfpq_indexing_gpu

INSTALL.md Outdated
Comment on lines 254 to 257
** update **
```
export PYTHONPATH=.
python demos/demo_auto_tune.py
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment, running the python tests involves the following:

$ make -C build swigfaiss
$ (cd build/faiss/python && setup.py build)
$ PYTHONPATH=./build/faiss/python/build/lib python -m unittest discover ./tests

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdouze has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@mdouze merged this pull request in 218a6a9.

@mdouze mdouze deleted the mdouze-update-INSTALL branch January 29, 2021 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants