Skip to content

Conversation

@kwasd
Copy link
Contributor

@kwasd kwasd commented Nov 3, 2024

Related to #2522

@kwasd kwasd marked this pull request as draft November 3, 2024 21:59
@kwasd kwasd changed the title Benchmarks core subset Benchmarks subset Nov 12, 2024
@kwasd kwasd marked this pull request as ready for review November 12, 2024 01:07
@kwasd kwasd requested a review from pbchekin November 12, 2024 01:08
@kwasd
Copy link
Contributor Author

kwasd commented Nov 12, 2024

- name: Run Triton GEMM kernel benchmark
if: ${{ steps.install.outcome == 'success' && !cancelled() }}
if: ${{ steps.install.outcome == 'success' && !cancelled() && !contains(inputs.skip_benchmarks, 'gemm_benchmark.py') }}
Copy link
Contributor

Choose a reason for hiding this comment

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

contains(inputs.skip_benchmarks, 'gemm_benchmark.py') returns true if the skip list contains gemm_benchmark.py_default or gemm_benchmark.py_advanced. Is it expected?

Copy link
Contributor Author

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 is true if the input is a JSON list, see the test run with ['softmax_kernel.py','gemm_benchmark.py']: https://github.com/intel/intel-xpu-backend-for-triton/actions/runs/11788616521/job/32836016215

Copy link
Contributor

Choose a reason for hiding this comment

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

How contains knows this is json and not a string? The input type iof skip_benchmarks is string and there is no fromJSON...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How contains knows this is json and not a string? The input type iof skip_benchmarks is string and there is no fromJSON...

I don't know, let's add this for clarity.

Copy link
Contributor Author

@kwasd kwasd Nov 12, 2024

Choose a reason for hiding this comment

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

Copy link
Contributor

@pbchekin pbchekin left a comment

Choose a reason for hiding this comment

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

Is it simpler to add input checkboxes for each benchmark?

@kwasd
Copy link
Contributor Author

kwasd commented Nov 12, 2024

Is it simpler to add input checkboxes for each benchmark?

I've tried, but number of inputs is limited to 10

Invalid workflow file: .github/workflows/triton-benchmarks.yml#L1
you may only define up to 10 `inputs` for a `workflow_dispatch` event

@vlad-penkin vlad-penkin linked an issue Nov 12, 2024 that may be closed by this pull request
Copy link
Contributor

@pbchekin pbchekin left a comment

Choose a reason for hiding this comment

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

Looks good! Let's merge and see how daily run work.

@kwasd
Copy link
Contributor Author

kwasd commented Nov 12, 2024

Thanks for your contribution!

@kwasd kwasd enabled auto-merge (squash) November 12, 2024 18:59
@kwasd kwasd merged commit e8b34a0 into main Nov 12, 2024
5 checks passed
@kwasd kwasd deleted the feature/benchmarks-subset branch November 12, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable periodic benchmarks for client GPUs

3 participants