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

[UT] add skiplist in pvc w/ lts for skipped cases #1156

Open
wants to merge 1 commit into
base: llvm-target
Choose a base branch
from

Conversation

AshburnLee
Copy link
Contributor

@AshburnLee AshburnLee commented May 20, 2024

Functionality

This PR add all SKIPPED FAILED cases to a skiplist in scripts/skiplist/pvc_lts/
language 112 failed cases
operators 17 failed cases

Solves issue #986

Result

Now there are no SKIPPED cases on PVC w/ LTS driver

Notes

1. This PR only skips SKIPPED cases, no FAILED cases
2. Agama version is captured form intel-level-zero-gpu, because I found the original way won't work on my local PVC w/ LTS, the change in my PR works.

@AshburnLee AshburnLee self-assigned this May 20, 2024
@AshburnLee
Copy link
Contributor Author

About the convert function from agama to driver type, I can not think of another way but hard code the agama version in the function. @vlad-penkin could you give some tips if this is not a good practice?

@vlad-penkin vlad-penkin linked an issue May 20, 2024 that may be closed by this pull request
@@ -0,0 +1,169 @@
# https://github.com/intel/intel-xpu-backend-for-triton/issues/986
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this skip list contains more test cases that the existing default skip list? The run (https://github.com/intel/intel-xpu-backend-for-triton/actions/runs/9214379486) shows that we do not need to skip more tests than the current skip list has.

@@ -50,3 +46,50 @@ if [ "$QUIET" = false ]; then
echo "AGAMA_VERSION=$AGAMA_VERSION"
echo "GPU_DEVICE=$GPU_DEVICE"
fi

agama_to_driver_type() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do not need to detect what driver (rolling or lts) is currently installed. Instead, a specific runner should set an environment variable either to select a skip list or to select a release stream (if you still need it for some reason).

Copy link
Contributor

Choose a reason for hiding this comment

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

@AshburnLee could you please address @pbchekin comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks~ I'm working on other PR's comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So from what I understand, in this PR, I just need to:

  1. remove agama_to_driver_type() and xpu_is_pvc(),
  2. Run test_triton.sh and collect FAILED cases from the log, and put those FAILED in a skiplist pvc_lts/....txt

Please confirm that I am right @pbchekin , thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And the automation script #1196 is not in this scope

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you can use test report from https://github.com/intel/intel-xpu-backend-for-triton/actions/runs/9507147050. This is a run where all errors are ignored and full test report is attached as artifact.

all: passed: 17460, failed: 129, skipped: 400, xfailed: 1240, total: 19229, fixme: 12 pass rate (w/o xfailed): 97.06%

Copy link
Contributor Author

Choose a reason for hiding this comment

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

working on it.

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~

@@ -50,3 +46,50 @@ if [ "$QUIET" = false ]; then
echo "AGAMA_VERSION=$AGAMA_VERSION"
echo "GPU_DEVICE=$GPU_DEVICE"
fi

agama_to_driver_type() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@AshburnLee could you please address @pbchekin comments?

@AshburnLee AshburnLee enabled auto-merge (squash) June 15, 2024 00:23
@AshburnLee AshburnLee disabled auto-merge June 15, 2024 00:25
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.

Add PVC skiplist for the LTS Driver
3 participants