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

ci: aarch64: Initial nightly performance test #2837

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Ryo-not-rio
Copy link
Contributor

@Ryo-not-rio Ryo-not-rio commented Mar 7, 2025

Adding a comprehensive performance test to the nightly pipeline. For now, matmul, conv, eltwise and reorders are tested but more tests are to be added in the future.

Other minor changes:

  • Fix benchdnn comparison script
  • Update precommit performance tests list
  • Add fast math tests

A run of the new pipeline can be seen in https://github.com/oneapi-src/oneDNN/actions/runs/13707641078. It is currently failing as it is catching a regression that needs to be solved but the pipeline itself is working as it should.

@Ryo-not-rio Ryo-not-rio requested a review from a team as a code owner March 7, 2025 11:51
@github-actions github-actions bot added the devops Github automation label Mar 7, 2025
Adding a comprehensive performance test to the nightly pipeline.
For now, matmul, conv, eltwise and reorders are tested but more tests
are to be added in the future.

Other minor changes:
- Fix benchdnn comparison script
- Update precommit performance tests list
- Add fast math tests
@Ryo-not-rio Ryo-not-rio force-pushed the ryo-not-rio/performance-test-ci branch from 64fa5ad to 6f1a913 Compare March 11, 2025 13:15
@@ -0,0 +1,46 @@
#! /bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a separate script for fast math? The BF16 paths should all be exercised by setting the --attr-fast-math benchdnn flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahhhh was not aware, good to know!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

"$2 --reorder --mode=P --perf-template=%prb%,%-time% --batch=${SCRIPT_DIR}/inputs/reorder_nightly >> $4"
)

for i in {1..5}
Copy link
Contributor

@Sqvid Sqvid Mar 12, 2025

Choose a reason for hiding this comment

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

I think we should make the number of runs a variable so it always stays in sync between the loop, and the print statements below.

for i in {1..5}
N = 5

for i in {1..$N}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this works when N is a variable. I think you want for i in $(seq ${N})

Copy link
Contributor

@Sqvid Sqvid Mar 13, 2025

Choose a reason for hiding this comment

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

I see you implemented the seq change but keep in mind by doing seq 0 $N rather than seq $N you have changed the number of loops from N to N+1. Was this intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

for i in {1..5}
N = 5

for i in {1..$N}
Copy link
Contributor

Choose a reason for hiding this comment

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

seq ${N} again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@Ryo-not-rio Ryo-not-rio force-pushed the ryo-not-rio/performance-test-ci branch from 7e3fc41 to ce967d5 Compare March 13, 2025 09:11
@Ryo-not-rio Ryo-not-rio requested a review from a team as a code owner March 13, 2025 09:11
@Ryo-not-rio Ryo-not-rio force-pushed the ryo-not-rio/performance-test-ci branch from ce967d5 to 54e3b69 Compare March 13, 2025 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops Github automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants