- 
                Notifications
    You must be signed in to change notification settings 
- Fork 285
chore(ci): set regression targets for cpu #2947
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
2d5baec    to
    c836091      
    Compare
  
    c836091    to
    2add781      
    Compare
  
    2add781    to
    6d97850      
    Compare
  
    | I need to add support for  | 
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.
A few questions/comments, also, why is it taking 2h+ to get those benchmarks in that case ?
@IceTDrinker reviewed 1 of 1 files at r1, 1 of 2 files at r2, 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @soonum)
ci/regression.toml line 84 at r3 (raw file):
"swap_claim::no_cmux" ] target.hlapi-erc20 = ["transfer::whitepaper", "transfer::no_cmux"]
why two variants here ?
ci/perf_regression/perf_regression.py line 634 at r3 (raw file):
try: op_perf = OperationPerformance(op_name, baseline_data, head_branch_value) except statistics.StatisticsError as err:
when did you encounter this issue ?
ci/perf_regression/perf_regression.py line 740 at r3 (raw file):
"<summary><strong>View All Benchmarks</strong></summary>", "", f"* Minor change threshold: +/- {MINOR_CHANGE_SCALE_FACTOR}x StdDev",
nice idea, do we print by how many std dev the latest value diverges ?
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.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @antoniupop and @IceTDrinker)
ci/regression.toml line 84 at r3 (raw file):
Previously, IceTDrinker wrote…
why two variants here ?
Because @antoniupop told me they use both of them in the coprocessor code.
ci/perf_regression/perf_regression.py line 634 at r3 (raw file):
Previously, IceTDrinker wrote…
when did you encounter this issue ?
When there is only one data point in main. Which is unlikely to happen except when we introduce a brand new benchmark (either new operation or a new params set)
ci/perf_regression/perf_regression.py line 740 at r3 (raw file):
Previously, IceTDrinker wrote…
nice idea, do we print by how many std dev the latest value diverges ?
Nope but I can add it easily. Should I do it ?
| 
 Previously, soonum (David Testé) wrote…
 ok yeah | 
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.
Thanks a lot ! :)
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @soonum)
ci/regression.toml line 84 at r3 (raw file):
Previously, soonum (David Testé) wrote…
Because @antoniupop told me they use both of them in the coprocessor code.
ok surprising which variant is set to be standardized in an ERC ?
ci/perf_regression/perf_regression.py line 740 at r3 (raw file):
Previously, soonum (David Testé) wrote…
Nope but I can add it easily. Should I do it ?
not necessarilyin this PR to keep the focus and avoid having this PR being delayed
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @soonum)
ci/regression.toml line 84 at r3 (raw file):
Previously, IceTDrinker wrote…
ok surprising which variant is set to be standardized in an ERC ?
No idea. Let's keep the two variants for now and remove the unused if they finally use only one.
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.
Reviewable status: all files reviewed, 1 unresolved discussion
ci/regression.toml line 84 at r3 (raw file):
Previously, soonum (David Testé) wrote…
No idea. Let's keep the two variants for now and remove the unused if they finally use only one.
let's have a backlog issue maybe to avoid forgetting about it ?

This change is