Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[CI] Add weights verification jobs #6996

Merged
merged 20 commits into from
Apr 13, 2023
Merged

[CI] Add weights verification jobs #6996

merged 20 commits into from
Apr 13, 2023

Conversation

s3krit
Copy link
Contributor

@s3krit s3krit commented Apr 3, 2023 •
edited
Loading

Adds two jobs:

  1. A job that runs after the runtime benchmarking jobs (which are manually triggered prior to a release) that checks that all of the weights files for a given runtime have been updated recently. This is currently set to a 2 day range, but could probably be shrunk to just $TODAY and the day before (to account for benchmark runs that take place overnight). Exampels of the runs can be found here: https://github.com/paritytech/polkadot/actions/runs/4681684403 - note that the jobs failed because they correctly identified that the weights have not been updated since the last release
  2. A job that uses https://github.com/ggwpez/substrate-weight-compare to compare the weights of a PR against the weights of the last release, commenting with some nicely-formatted tables on any newly added weights, and changes changes in pre-existing weights. See the bottom of this PR for examples of the output.

@s3krit s3krit added A3-in_progress Pull request is in progress. No review needed at this stage. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Apr 3, 2023
@s3krit
Copy link
Contributor Author

s3krit commented Apr 3, 2023

bot rebase

@paritytech paritytech deleted a comment from paritytech-processbot bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 4, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 5, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 5, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 5, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 5, 2023
@paritytech-ci paritytech-ci requested a review from a team April 12, 2023 19:04
@s3krit s3krit requested review from ggwpez and removed request for chevdor April 12, 2023 19:04
alvicsam
alvicsam reviewed Apr 13, 2023
View reviewed changes
.github/workflows/check-weights.yml
- name: Check weight changes
shell: bash
run: |
cargo install --git https://github.com/ggwpez/substrate-weight-compare swc
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's worth to pin version of the package

Copy link
Member

Choose a reason for hiding this comment

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

You can just add --rev 30d1cd6b33ecf2dac32d48cb277f9b4d2d69786a (latest master commit) if you are not a docker fan.

@paritytech-ci paritytech-ci requested a review from a team April 13, 2023 06:24
@paritytech-ci paritytech-ci requested a review from a team April 13, 2023 08:43
ggwpez
ggwpez approved these changes Apr 13, 2023
View reviewed changes
Copy link
Member

@ggwpez ggwpez left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, thanks!

It does actually confirm that the bench script is broken:

pallet_balances.rs was not updated for the current date. Last update: 2022-11-15
pallet_balances_nis_counterpart_balances.rs was not updated for the current date. Last update: 2023-03-15

since it cannot deal with multiple instance pallets… basically this line forces the output into the wrong file. It should print a warning in that case.

scripts/ci/github/verify_updated_weights.sh
@@ -0,0 +1,56 @@
#!/bin/bash
Copy link
Member

@ggwpez ggwpez Apr 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

One thing that comes to my mind is that we probably also want this in cumulus, so could be that the scripts could also live in https://github.com/paritytech/pipeline-scripts/

Although on the other hand we should "soon" have our monorepo, in which case it needs to be shuffled around anyway.

@s3krit
revert docker experiment
277c7a3 
too janky. will fix upstream stuff and fix as a separate PR

reverts commits:

- 3be2043
- fd17a0f
- 59f0ebf
@paritytech paritytech deleted a comment from github-actions bot Apr 13, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 13, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 13, 2023
@paritytech paritytech deleted a comment from github-actions bot Apr 13, 2023
@s3krit
Copy link
Contributor Author

s3krit commented Apr 13, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@s3krit
Copy link
Contributor Author

s3krit commented Apr 13, 2023

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit e164da6 into master Apr 13, 2023
@paritytech-processbot paritytech-processbot bot deleted the mp-weights-verification branch April 13, 2023 15:28
@s3krit s3krit mentioned this pull request Apr 13, 2023
Closed
@ggwpez ggwpez mentioned this pull request Apr 14, 2023
Merged
@ggwpez
Copy link
Member

ggwpez commented Apr 14, 2023

So how is the process for this @coderobe ?
It looks like they should run every time when a release happens and the weights got updated. I just want to ensure that we dont ignore these errors (too often).
Could you then please ping me when it fails on the next release?

@ggwpez ggwpez mentioned this pull request Apr 27, 2023
Closed
15 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ggwpez ggwpez ggwpez approved these changes

@alvicsam alvicsam alvicsam approved these changes

@coderobe coderobe coderobe approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@s3krit @ggwpez @coderobe @alvicsam