-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
BENCH pin dependencies in asv benchmarks #27264
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
Conversation
I think it makes sense to do this. What do you think of documenting somewhere close to where the other lock files are generated that the benchmark dependencies are controlled from somewhere else? Worth it? I'm -1 on trying to automated it or also control the benchmark dependencies from the other lock file generation stuff, but unsure about documentation. Maybe ASV enthusiasts know and expect that dependencies are set in the config file (which would make it redundant to document it elsewhere) |
Well I'm not sure where to document that. We don't necessarily want to update these ones at the same time that we udate the lock files. But on the other hand there's a risk to forget to update them for a while if we don't write it somewhere. |
I don't have a great idea where to put the documentation either. So maybe let's not for now unless someone comments with a smart idea. |
Building on the proposal of Tim in #27264 (comment), if #27266 is approved, we can add a comment next to the Cython specification indicating that the asv configuration might need to be updated as well. Unrelated: Generally, I think we would be nice to have "Community Reminders", indicating updates or changes that we might want to perform at given frequencies. |
I added a comment in the asv conf, it doesn't hurt. |
related to #27038 |
I'll enable auto-merge for this. |
From the discussions in #27086
Currently the asv benchmarks run against the latest available versions of the dependencies.
This PR allows to easily distinguish between perf regressions due to a change in the codebase from perf regressions due to an upgrade of some dependency. This way, bumping a dependency in the benchmarks will correspond to a specific commit in the history.
For now I pinned cython < 3 because of the perf regressions we're observing in #27086. Then I propose to make a new PR after that bumping to cython 3 to have a specific commit identifying the cython 3 regressions.