Add resnet and llama to benchmark models #1386
Conversation
vcanicTT
requested review from
vmilosevic,
nvukobratTT,
pilkicTT and
dgolubovicTT
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1386 +/- ##
=======================================
Coverage 43.40% 43.40%
=======================================
Files 48 48
Lines 7860 7860
=======================================
Hits 3412 3412
Misses 4448 4448 ☔ View full report in Codecov by Sentry. |
|
@dgolubovicTT @pilkicTT @vmilosevic Can you take a look? |
|
||||||||||||||
1 similar comment
|
||||||||||||||
|
||||||||||||||
1 similar comment
|
||||||||||||||
| @@ -8,4 +8,10 @@ | |||
|
|
|||
|
|
|||
| # MNIST Linear | |||
| python forge/test/benchmark/benchmark.py -m mnist_linear -bs 1 -lp 32 -o forge-benchmark-e2e-mnist.json | |||
| PYTHONPATH=./forge python forge/test/benchmark/benchmark.py -m mnist_linear -bs 1 -lp 32 -o forge-benchmark-e2e-mnist.json | |||
There was a problem hiding this comment.
Why is the PYTHONPATH set? We should avoid this...
There was a problem hiding this comment.
I had problems with python packages and imports, and this was the best way to solve the problem. When I run the tests via benchmark script it doesn't see tests module. Do you have any suggestions how to resolve this in a better way?
.github/workflows/perf-benchmark.yml
Outdated
| @@ -82,7 +82,9 @@ jobs: | |||
| shell: bash | |||
| run: | | |||
| source env/activate | |||
| python forge/test/benchmark/benchmark.py -m mnist_linear -bs 1 -lp 32 -o ${{ steps.strings.outputs.perf_report_path }} | |||
| PYTHONPATH=./forge python forge/test/benchmark/benchmark.py -m mnist_linear -bs 1 -lp 32 -o ${{ steps.strings.outputs.perf_report_path }} | |||
There was a problem hiding this comment.
It would be good to run all the models with higher batch size.
There was a problem hiding this comment.
The tests can be run in batches larger than one. For now, we keep it at one to ensure the models work correctly. We can increase it later when we start tracking results and are confident that they are preserved accurately.
Yes, I sent a PR only for ResNet. Then, I rebased on the ResNet branch and continued working on LLaMA. If we do everything in this PR, I will close the first one. |
vcanicTT
force-pushed
the
vcanic/bringup_benchmark_llama
branch
from
24b220f to
c505238
Compare
| @@ -44,7 +44,10 @@ jobs: | |||
| run: | | |||
| echo "work-dir=$(pwd)" >> "$GITHUB_OUTPUT" | |||
| echo "build-output-dir=$(pwd)/build" >> "$GITHUB_OUTPUT" | |||
| echo "perf_report_path=forge-benchmark-e2e-mnist_$JOB_ID.json" >> "$GITHUB_OUTPUT" | |||
There was a problem hiding this comment.
@vmilosevic Can you look at this part, this file, and please tell if you're ok or not with this change? A model expects .json file as output, so I made three different paths for each model, and one path that represents folder from which we upload the files, i.e. benchmark reports.
|
||||||||||||||
|
||||||||||||||
|
||||||||||||||
|
||||||||||||||
We want to track the end-to-end performance of different models, so we have added ResNet HF and Llama Prefill to the benchmark models. This PR also includes a few changes to existing models, aiming to align all models to be written in the same manner and uploaded successfully.