Skip to content

Conversation

@lqwk
Copy link
Contributor

@lqwk lqwk commented Aug 25, 2022

Introduce RuntimeSeriesReward

Introduce a new implementation of comparing program runtimes that computes the reward as the difference of the medians between the current set of runtimes and the previous set of runtimes only if the runtime series are significantly different (determined by the Kruskal–Wallis test).

Source: https://htor.inf.ethz.ch/publications/img/hoefler-scientific-benchmarking.pdf

Testing

I ran a series of tests comparing the new implementation with the existing implementation using the LLVM autotuner on the cbench-v1 benchmark. The rewards are shown below:

Benchmark Runtime Runtime Series
cbench-v1/bitcount 0.847564 0.780826
cbench-v1/blowfish 0.997087 0.997496
cbench-v1/bzip2 2.505352 2.516984
cbench-v1/crc32 1.003222 0.995800
cbench-v1/dijkstra 1.001125 1.014525
cbench-v1/gsm 0.785371 0.812009
cbench-v1/jpeg-c 0.976613 0.989570
cbench-v1/jpeg-d 1.004371 1.010437
cbench-v1/patricia 0.992379 1.010707
cbench-v1/qsort 0.958153 0.986507
cbench-v1/sha 1.000886 0.990302
cbench-v1/stringsearch 1.004281 1.006058
cbench-v1/stringsearch2 1.009237 1.050334
cbench-v1/susan 0.959058 0.975618
cbench-v1/tiff2bw 1.016761 1.018933
cbench-v1/tiff2rgba 1.105831 1.104215
cbench-v1/tiffdither 1.042336 1.022278
cbench-v1/tiffmedian 1.005050 1.010557

The new implementation is on par with the existing implementation, and even beats the existing implementation on 12/17 of the benchmarks.

I am proposing to merge this upstream and we can maybe work on other optimizations such as early stopping.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 25, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2022

Codecov Report

❌ Patch coverage is 34.00000% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.63%. Comparing base (1c40e5b) to head (a53276a).
⚠️ Report is 134 commits behind head on development.

Files with missing lines Patch % Lines
compiler_gym/spaces/runtime_series_reward.py 28.57% 25 Missing ⚠️
compiler_gym/wrappers/llvm.py 33.33% 8 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (1c40e5b) and HEAD (a53276a). Click for more details.

HEAD has 43 uploads less than BASE
Flag BASE (1c40e5b) HEAD (a53276a)
45 2
Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##           development     #762       +/-   ##
================================================
- Coverage        89.29%   55.63%   -33.67%     
================================================
  Files              130      131        +1     
  Lines             7912     7961       +49     
================================================
- Hits              7065     4429     -2636     
- Misses             847     3532     +2685     
Files with missing lines Coverage Δ
compiler_gym/spaces/__init__.py 100.00% <100.00%> (ø)
compiler_gym/wrappers/__init__.py 100.00% <100.00%> (ø)
compiler_gym/wrappers/llvm.py 44.82% <33.33%> (-55.18%) ⬇️
compiler_gym/spaces/runtime_series_reward.py 28.57% <28.57%> (ø)

... and 87 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lqwk
Copy link
Contributor Author

lqwk commented Aug 25, 2022

Note: needs #761 to land first

@ChrisCummins
Copy link
Contributor

Hi @lqwk, I'm very sorry for my delay in reviewing this. I've not forgotten about it, I just have a backlog of issues to fix on the CI so that I can run the tests against these changes.

Cheers,
Chris

@ChrisCummins ChrisCummins added this to the v0.2.6 milestone Nov 2, 2022
@ChrisCummins
Copy link
Contributor

Hi @lqwk, okay, I finally pushed through the backlog of issues and have a newly stable v0.2.5 release. Sorry again that it me so long to getting around to this.

Are you still working on this? If so, could you please rebase this on top of the development branch so that we can use the CI to verify that all tests pass.

Cheers,
Chris

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants