-
Notifications
You must be signed in to change notification settings - Fork 81
Conversation
Oops I forgot to update the README and the |
852a076
to
97a9f6c
Compare
377d7ea
to
0cd6478
Compare
Co-authored-by: Clémentine Urquizar <clementine@meilisearch.com>
0cd6478
to
9a920d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you Tamo!!
@Kerollmops, should I merge this in the #322, or should we first merge the geosearch and then the benchmarks on main? What do you prefer? |
I prefer that we merge #322 and this one after :) |
Ok so I guess we can merge now 😁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Have you tested it with the CI, already?
Yes! |
How much time does it take to run this one? If it is much longer than the other ones, maybe we should indeed! |
No, currently, all the bench related to the geosearch are super fast. It takes 20 minutes for the search and an insignificant amount of time in the indexing benchmark (1h50m5s before the geosearch vs 1h50m6s with this branch). So I think it would be good to keep this dataset of 1M documents because it's what we want to improve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can open a new PR to add a small dataset if you want, I prefer to merge this one for now :)
bors merge
closes #336
Should I merge this PR in #322 and then we merge everything in
main
or should we wait for #322 to be merged and then merge this one inmain
later?