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.

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

bench bot subweight output tuning #7602

Open
bkontur opened this issue Feb 18, 2025 · 1 comment
Open

bench bot subweight output tuning #7602

bkontur opened this issue Feb 18, 2025 · 1 comment
Labels
T12-benchmarks This PR/Issue is related to benchmarking and weights.

Comments

@bkontur
Copy link
Contributor

bkontur commented Feb 18, 2025

Problem

After running the bench bot, we execute subweight to compare the fresh weight files with the base branch. Currently, this base branch is hardcoded as refs/remotes/origin/master for polkadot-sdk and refs/remotes/origin/main for polkadot-fellows.

This setup works fine when the bench bot is used on a pull request targeting master or main within the same repository. However, it has several downsides, as it can produce misleading results:

  • If the PR originates from a fork, the comparison is made against the fork’s master or main, which is incorrect.
  • If the PR’s target/base branch is different from master or main, issues arise. For example, if I have one PR (origin/pr1origin/master) and then another PR (origin/pr2origin/pr1), running the bench bot in the second PR should compare origin/pr2 with origin/pr1.

Potential Solution

Instead of using hardcoded references to refs/remotes/origin/master (polkadot-sdk) and refs/remotes/origin/main (polkadot-fellows), we should dynamically determine the PR’s actual target/base branch.

This can potentially be achieved using some git remote add scripting along with fetching PR data via the GitHub REST API, as referenced here.

@bkontur bkontur added the T12-benchmarks This PR/Issue is related to benchmarking and weights. label Feb 18, 2025
@bkontur
Copy link
Contributor Author

bkontur commented Feb 18, 2025

cc: @mordamax

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T12-benchmarks This PR/Issue is related to benchmarking and weights.
Projects
None yet
Development

No branches or pull requests

1 participant