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.

Sign up for GitHub

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

Jump to bottom

test: fix benchmark tests #173

Merged
merged 1 commit into from
Sep 20, 2021
Merged

test: fix benchmark tests #173

merged 1 commit into from
Sep 20, 2021

Conversation

cristianmtr
Copy link
Contributor

@cristianmtr cristianmtr commented Sep 10, 2021 •
edited
Loading

Various fixes for the PSQLFaiss benchmarks and tests

Also fixes #208

@cristianmtr cristianmtr force-pushed the fix-psql-benchmarks branch 4 times, most recently from 1085c68 to dc5c5f8 Compare September 20, 2021 07:03
@cristianmtr cristianmtr marked this pull request as ready for review September 20, 2021 07:03
maximilianwerk
maximilianwerk requested changes Sep 20, 2021
View reviewed changes
Copy link
Member

@maximilianwerk maximilianwerk left a comment

Choose a reason for hiding this comment

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

I am not super happy about the non-executor specific integration tests. Now, that we have the FaissPostgres combination we might want to outsource all the dump-reload mechanism testing towards only this indexer soon. (not in this PR)

jinahub/indexers/searcher/compound/FaissPostgresSearcher/faisspsql.py Outdated Show resolved Hide resolved
jinahub/indexers/searcher/compound/FaissPostgresSearcher/tests/unit/test_unit.py Outdated Show resolved Hide resolved
tests/requirements.txt Outdated Show resolved Hide resolved
@cristianmtr cristianmtr force-pushed the fix-psql-benchmarks branch 2 times, most recently from f2de150 to 127ff4e Compare September 20, 2021 07:33
@cristianmtr cristianmtr force-pushed the fix-psql-benchmarks branch 4 times, most recently from d2257ff to de5542b Compare September 20, 2021 08:17
maximilianwerk
maximilianwerk previously approved these changes Sep 20, 2021
View reviewed changes
Copy link
Member

@maximilianwerk maximilianwerk left a comment

Choose a reason for hiding this comment

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

LGTM

jinahub/indexers/searcher/compound/FaissPostgresSearcher/tests/integration/test_208.py Outdated Show resolved Hide resolved
@cristianmtr cristianmtr force-pushed the fix-psql-benchmarks branch 2 times, most recently from a83c181 to 900c6ae Compare September 20, 2021 08:43
maximilianwerk
maximilianwerk approved these changes Sep 20, 2021
View reviewed changes
Copy link
Member

@maximilianwerk maximilianwerk left a comment

Choose a reason for hiding this comment

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

LGTM

@cristianmtr cristianmtr merged commit d1d7444 into main Sep 20, 2021
@cristianmtr cristianmtr deleted the fix-psql-benchmarks branch September 20, 2021 09:24
@cristianmtr cristianmtr mentioned this pull request Sep 21, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maximilianwerk maximilianwerk maximilianwerk approved these changes

@tadejsv tadejsv Awaiting requested review from tadejsv

@numb3r3 numb3r3 Awaiting requested review from numb3r3

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug FaissPostgress - int not parsed
2 participants
@cristianmtr @maximilianwerk