Skip to content

Conversation

@victor-eds
Copy link
Contributor

@victor-eds victor-eds commented Aug 24, 2023

Rename the workflow file so that our own SYCL MLIR Pre Commit on Linux is triggered instead of SYCL Pre Commit on Linux.

The latter lead to an additional job testing SYCL-MLIR on CUDA devices (always failing).

Note: Future changes in sycl branch CI may lead to conflicts when merging.

@victor-eds victor-eds added the sycl-mlir Pull requests or issues for sycl-mlir branch label Aug 24, 2023
@victor-eds victor-eds self-assigned this Aug 24, 2023
@victor-eds victor-eds changed the title Test [SYCL-MLIR][CI] Trigger SYCL MLIR Pre Commit on Linux workflow Aug 24, 2023
@victor-eds victor-eds marked this pull request as ready for review August 24, 2023 12:44
@victor-eds victor-eds requested a review from bader August 24, 2023 12:44
@victor-eds
Copy link
Contributor Author

@bader you were totally right. I checked what was going on and, apparently, despite naming a different workflow in the .yml file, the SYCL Pre Commit on Linux was being triggered. This change triggers our own workflow, so no additional jobs like the CUDA AWS one will be triggered.

@whitneywhtsang
Copy link
Contributor

The latter lead to an additional job testing SYCL-MLIR on CUDA devices (always failing).

Why .github/workflows/sycl_precommit_linux.yml would trigger CUDA testing? Where is that define?

@victor-eds
Copy link
Contributor Author

Why .github/workflows/sycl_precommit_linux.yml would trigger CUDA testing? Where is that define?

Nowhere to be found. I'm no CI expert, but, looking at previous jobs, it seems our workflow was being run with a different name (?).

@jsji
Copy link
Contributor

jsji commented Aug 24, 2023

The latter lead to an additional job testing SYCL-MLIR on CUDA devices (always failing).

Why .github/workflows/sycl_precommit_linux.yml would trigger CUDA testing? Where is that define?

My understanding is: https://github.com/intel/llvm/blob/sycl/.github/workflows/sycl_precommit_aws.yml use

workflow_run:
workflows: [SYCL Pre Commit on Linux]

So as long as the workflow name pre commit testing is SYCL Pre Commit on Linux, it will trigger the CUDA testing in sycl_precommit_aws.yml.

And according to https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows

Note: This event will only trigger a workflow run if the workflow file is on the default branch.

So changing the workflow name in sycl_precommit_aws.yml in SYCL-MLIR branch won't help, as it is not on the default branch.

So yeah, it is good idea to rename the workflow for SYCL-MLIR branch to something else other than SYCL Pre Commit on Linux.

@victor-eds
Copy link
Contributor Author

So yeah, it is good idea to rename the workflow for SYCL-MLIR branch to something else other than SYCL Pre Commit on Linux.

We already had:

name: SYCL MLIR Pre Commit on Linux

but our workflow appeared with the SYCL Pre Commit on Linux name when looked in this project's actions (not in CI report in the PRs, tho). My crazy bet is that the name was taken from the sycl branch file due to the filename collision.

@whitneywhtsang
Copy link
Contributor

our workflow appeared with the SYCL Pre Commit on Linux name when looked in this project's actions

Where do you see that?
https://github.com/intel/llvm/actions/runs/5962217402/workflow?pr=10952 also appeared as SYCL MLIR Pre Commit on Linux.

@victor-eds
Copy link
Contributor Author

victor-eds commented Aug 24, 2023

our workflow appeared with the SYCL Pre Commit on Linux name when looked in this project's actions

Where do you see that? https://github.com/intel/llvm/actions/runs/5962217402/workflow?pr=10952 also appeared as SYCL MLIR Pre Commit on Linux.

Here you can see jobs triggered by PRs both targeting sycl and sycl-mlir, and here, just the job triggered by this PR making the change. If this is intended GitHub behavior, it's a bit obscure for the untrained eye, TBH.

@bader bader requested review from aelovikov-intel and removed request for bader August 24, 2023 15:13
@aelovikov-intel
Copy link
Contributor

our workflow appeared with the SYCL Pre Commit on Linux name when looked in this project's actions

Where do you see that? https://github.com/intel/llvm/actions/runs/5962217402/workflow?pr=10952 also appeared as SYCL MLIR Pre Commit on Linux.

Here you can see jobs triggered by PRs both targeting sycl and sycl-mlir, and here, just the job triggered by this PR making the change. If this is intended GitHub behavior, it's a bit obscure for the untrained eye, TBH.

I can't tell enough how I hate Github Actions after working with them very closely for the last half a year. Everything looks cumbersome, unfinished and buggy. The developers seem to jump from one thing to another without polishing anything for the release quality...

@victor-eds
Copy link
Contributor Author

I can't tell enough how I hate Github Actions after working with them very closely for the last half a year. Everything looks cumbersome, unfinished and buggy. The developers seem to jump from one thing to another without polishing anything for the release quality...

This looks definitely like a bug to me (maybe they'll document it and make it a feature now 😆). Thanks for the reviews guys, CI can be painful.

@victor-eds victor-eds merged commit 83d60d8 into intel:sycl-mlir Aug 24, 2023
@victor-eds victor-eds deleted the ci-test branch August 24, 2023 16:31
aelovikov-intel pushed a commit that referenced this pull request Aug 28, 2023
…#10926)

Revert commit 4e737e2 as dropping the
job will be handled in the `sycl-mlir` branch instead (see #10956).

Signed-off-by: Victor Perez <victor.perez@codeplay.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sycl-mlir Pull requests or issues for sycl-mlir branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants