[DRAFT] Restructuring the code #383

erikbern / ann-benchmarks

Benchmarks of approximate nearest neighbor libraries in Python

http://ann-benchmarks.com

MIT License

4.74k stars 718 forks source link

[DRAFT] Restructuring the code #383 #415

Closed Jeadie closed 1 year ago

Jeadie commented 1 year ago

Changing code structure for #383

Still in draft, fielding consenus/feedback.

Further refactors will be, but not included in this:

Refactor config.yml to have a default tag for common params.
Use FROM ann-benchmarks-abc for algorithm XYZ when applicable

erikbern commented 1 year ago

This is awesome! Let me know how we can get this merged. Think this structure is a lot cleaner.

Jeadie commented 1 year ago

No problem. I will try getting this ready for merge this week (to avoid ongoing merge conflicts). Currently working on this. Right now what is left to do it just minimally reimplement definition.py. I think there's better future refactors for this, but I want this PR to provide the least amount of logic changes.

Jeadie commented 1 year ago

Okay, changes are up. I have had a few problems with some of the tests in benchmarks.yml. All except for sptag are transient, that is they have at least one (generally more) passing attempts in my action history (once completing the featureset, of course). SPTAG seems to be the one consistently to fail.

Jeadie commented 1 year ago

@erikbern fixes in for restructure. Hopefully we can get it merged before more conflicts.

GH action run. SPTAG failing for build problems.

erikbern commented 1 year ago

This looks AMAZING!! I Would love to merge this asap

Sorry for being slow to check in on this – I've been super busy. Let me know if I can help with this in any way?

Jeadie commented 1 year ago

No problem, I understand early startup days can be intense. I often get transient or odd errors in various algorithm specific tests. Will just want to make sure either a) you know the issue with a specific test, or b) you accept they won't be working in this PR run and we can work on improving/fixing post-merge.

erikbern commented 1 year ago

Let's merge this – I think it puts it in a better place for further work! Thanks a lot for this contribution!!!