Skip to content

Adding recall testing to openAI track#702

Merged
benwtrent merged 8 commits intoelastic:masterfrom
benwtrent:openai-recall-test
Mar 24, 2025
Merged

Adding recall testing to openAI track#702
benwtrent merged 8 commits intoelastic:masterfrom
benwtrent:openai-recall-test

Conversation

@benwtrent
Copy link
Member

This adds recall and greatly adjusts the OpenAI track.

I thinking we should do something similar with ALL our vector tracks, even for the dense vector track as recalculating the true knn, even for dense vector is wasted compute time.

This is a draft as its a hack for some local testing, if we really want to move forward with this, I can clean this up and we can commit it.

@benwtrent benwtrent marked this pull request as ready for review December 19, 2024 14:50
Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM

@gareth-ellis gareth-ellis requested a review from a team March 12, 2025 10:27
Copy link
Member

@gareth-ellis gareth-ellis left a comment

Choose a reason for hiding this comment

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

LGTM, with the exception that it adds 11 minutes to the system tests - this is because of the way the runner initialises, even in test mode.

I don't have a good solution right now - we have another track that does the same, we got around it by skipping in the system tests - like here #708

I dont think this will run for serverless, so just needs adding to the skip list for standard it.

One possibility, that I need to try, is to create a second file that contains many less queries, and use that based on a parameter being set. Then we could add a variant where we run e.g msmarco and openai setting this special parameter. I will add an issue so we don't forget about this tech debt...

Issue here: #754

Co-authored-by: Gareth Ellis <gareth.ellis@elastic.co>
@benwtrent
Copy link
Member Author

@gareth-ellis ah, I could add two "test mode" files? I have seen that done in other rally tracks.

@benwtrent
Copy link
Member Author

Let me know if I need to make any changes @gareth-ellis.

But, I do think once we get this finished, we can then make progress of moving our nightlies to this benchmark with different parameters.

@gareth-ellis
Copy link
Member

When you wrote earlier, I started looking at how we could do this - i've got it semi working in msmarco track, if i get it working 100% by later today, i'll try and apply the same to yours, otherwise will temporarily add your new track to the skip list and then we should be good

@gareth-ellis gareth-ellis self-requested a review March 13, 2025 13:52
@benwtrent benwtrent merged commit 000eeb7 into elastic:master Mar 24, 2025
13 checks passed
@benwtrent benwtrent deleted the openai-recall-test branch March 24, 2025 13:07
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.

3 participants

Comments