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

benchdnn: graph: check partition number by default #2314

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

Conversation

TaoLv
Copy link
Contributor

@TaoLv TaoLv commented Dec 24, 2024

Currently on main branch, --expected-n-partitions is 0 by default (if not provided in benchdnn command line), which means to skip checking the partition number returned from an input graph.
This PR changes the default value of --expected-n-partitions to 1 which means the input graph should be fused into one partition.
Most of the cases we have in CI/Nightly should fused into one partition. With this check enabled by default, we can detect that if a PR breaks a fusion into multiple partitions. Before this change, those multiple partitions will silently pass the correctness check of benchdnn.
Apparently, we still have a few cases which cannot fuse into one partition due to:

  • the cases previously were added for graph compiler backend.
  • the cases can fuse on cpu but not on gpu, or vice versa.

Partition number check is skipped for these cases explicitly by setting --expected-n-partitions=0 in the command lines. We can track and remove these settings when the fusions are implemented in the future.

@TaoLv TaoLv requested review from a team as code owners December 24, 2024 10:27
@github-actions github-actions bot added component:graph-api Codeowner: @oneapi-src/onednn-graph component:tests Codeowner: @oneapi-src/onednn-arch labels Dec 24, 2024
@TaoLv TaoLv force-pushed the lvtao/main/benchdnn-partition-num branch from 5806482 to b52dae0 Compare December 24, 2024 10:31
@TaoLv TaoLv requested a review from a team as a code owner December 24, 2024 10:31
@github-actions github-actions bot added the documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc label Dec 24, 2024
@TaoLv TaoLv force-pushed the lvtao/main/benchdnn-partition-num branch from b52dae0 to b182142 Compare December 24, 2024 10:34
@TaoLv
Copy link
Contributor Author

TaoLv commented Dec 24, 2024

make test
enable benchdnn_nightly
disable benchdnn_all
enable benchdnn_graph

tests/benchdnn/graph/graph.cpp Outdated Show resolved Hide resolved
@TaoLv TaoLv force-pushed the lvtao/main/benchdnn-partition-num branch from b182142 to 3e2d523 Compare December 27, 2024 11:16
@TaoLv TaoLv force-pushed the lvtao/main/benchdnn-partition-num branch from 3e2d523 to c6fc6d8 Compare December 27, 2024 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:graph-api Codeowner: @oneapi-src/onednn-graph component:tests Codeowner: @oneapi-src/onednn-arch documentation A request to change/fix/improve the documentation. Codeowner: @oneapi-src/onednn-doc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants