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

[NeurIPS 2023 Filter Track] faissplus #222

Merged
merged 20 commits into from
Nov 1, 2023

Conversation

PUITAR
Copy link
Contributor

@PUITAR PUITAR commented Oct 31, 2023

Our submission of algorithm 1: faissplus.

Using some parameters to optimize faiss.

@maumueller maumueller self-requested a review October 31, 2023 11:22
@maumueller
Copy link
Collaborator

maumueller commented Oct 31, 2023

@PUITAR This seems to have the same problems as mentioned in #223 . Please update your pull request to avoid changing the existing code base.

Copy link
Collaborator

@maumueller maumueller left a comment

Choose a reason for hiding this comment

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

add a file include in need.
@maumueller
Copy link
Collaborator

I'll try again, but please also fix the other issues where you remove files from our codebase.

@maumueller
Copy link
Collaborator

@PUITAR
Copy link
Contributor Author

PUITAR commented Oct 31, 2023

It still doesn't work: https://github.com/harsha-simhadri/big-ann-benchmarks/actions/runs/6706568601/job/18223474904?pr=222#step:4:1858

I‘m sorry we have so many problems with our code. I have just add a file to our faissplus, so run the action of ’install requirement‘ again may works.

@PUITAR PUITAR requested a review from maumueller October 31, 2023 15:23
@maumueller
Copy link
Collaborator

maumueller commented Oct 31, 2023

@PUITAR I don't see a new commit after the failing CI run that I am pointing to.

@PUITAR
Copy link
Contributor Author

PUITAR commented Nov 1, 2023

@maumueller I just realized some mistake. Since you might used the same docker to run the CI test, it wrong again.
you should see this, in the INTALL REQUIREMENT action of the CI test, there is an error

------
Dockerfile:15
--------------------
  13 |     RUN python3 -m pip install -r requirements_conda.txt 
  14 |     
  15 | >>> COPY neurips23/filter/faiss/bow_id_selector.swig ./
  16 |     
  17 |     RUN swig -c++ -python -I$CONDA_PREFIX/include -Ifaiss bow_id_selector.swig 
--------------------
ERROR: failed to solve: failed to compute cache key: failed to calculate checksum of ref 5de61ec8-c85c-4b78-898d-5c657cc86999::96cbz8xkkinfbz3umqxm8tzf3: "/neurips23/filter/faiss/bow_id_selector.swig": not found

This means the file(bow_id_selector.swig) lost, and i had add the file to my PR. So please run the intall requirement again. Thank you very much. orz

@maumueller
Copy link
Collaborator

@PUITAR This looks much better.

I got the following results:

faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 30, 'mt_threshold': 0.00033}))",yfcc-10M,10,3840.5696040105754,0.0,573.4747545719147,4905644.0,0,0,filter,0.895063
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 32, 'mt_threshold': 0.0003}))",yfcc-10M,10,3764.112198957016,0.0,573.4747545719147,4905644.0,0,0,filter,0.8968999999999999
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 32, 'mt_threshold': 0.00031}))",yfcc-10M,10,3756.703088341436,0.0,573.4747545719147,4905644.0,0,0,filter,0.897869
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 32, 'mt_threshold': 0.00033}))",yfcc-10M,10,3737.604998681685,0.0,573.4747545719147,4905644.0,0,0,filter,0.8996660000000001
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 34, 'mt_threshold': 0.0003}))",yfcc-10M,10,3673.341554167496,0.0,573.4747545719147,4905644.0,0,0,filter,0.901073
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 32, 'mt_threshold': 0.00035}))",yfcc-10M,10,3735.4637292410794,0.0,573.4747545719147,4905644.0,0,0,filter,0.901529
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 34, 'mt_threshold': 0.00031}))",yfcc-10M,10,3652.6801971503146,0.0,573.4747545719147,4905644.0,0,0,filter,0.902008
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 34, 'mt_threshold': 0.00033}))",yfcc-10M,10,3655.287962959017,0.0,573.4747545719147,4905644.0,0,0,filter,0.903754
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 34, 'mt_threshold': 0.00035}))",yfcc-10M,10,3635.117445737324,0.0,573.4747545719147,4905644.0,0,0,filter,0.9055630000000001
faissplus,"Faiss(('IVF11264,SQ8', {'nprobe': 40, 'mt_threshold': 0.0003}))",yfcc-10M,10,3453.915576495494,0.0,573.4747545719147,4905644.0,0,0,filter,0.9119970000000001

Is this ready to be merged?

@maumueller maumueller self-assigned this Nov 1, 2023
@maumueller maumueller merged commit 8313916 into harsha-simhadri:main Nov 1, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants