Add test_all_subset_models mode #2428
Conversation
|
Test runs (are still pending): |
.github/workflows/e2e-reusable.yml
Outdated
| elif [[ "${{ inputs.models }}" == "subset" ]]; then | ||
| while read model; do | ||
| bash -e $GITHUB_WORKSPACE/scripts/inductor_xpu_test.sh ${{ inputs.suite }} ${{ inputs.dtype }} ${{ inputs.mode }} ${{ inputs.test_mode }} xpu 0 static 1 0 $model | ||
| bash -e $GITHUB_WORKSPACE/scripts/inductor_xpu_test.sh ${{ inputs.suite }} ${{ inputs.dtype }} ${{ inputs.mode }} ${{ inputs.test_mode }} xpu 0 static 1 0 $model || ${{ inputs.test_all_subset_models }} |
There was a problem hiding this comment.
This won't work as expected, the script returns 0 even if some accuracy tests or performance runs failed. You need to check the report (csv file) and verify it has all items from a corresponding subset and every item has passed. See https://github.com/intel/intel-xpu-backend-for-triton/blob/main/.github/workflows/build-test-reusable.yml#L179-L181 for the idea.
Co-authored-by: Pavel Chekin <[email protected]>
Co-authored-by: Pavel Chekin <[email protected]>
…ure/2246-e2e-accuracy-subset
…/intel/intel-xpu-backend-for-triton into feature/2246-e2e-accuracy-subset
.github/workflows/e2e-reusable.yml
Outdated
| elif [[ "${{ inputs.models }}" == "subset" ]]; then | ||
| while read model; do | ||
| bash -e $GITHUB_WORKSPACE/scripts/inductor_xpu_test.sh ${{ inputs.suite }} ${{ inputs.dtype }} ${{ inputs.mode }} ${{ inputs.test_mode }} xpu 0 static 1 0 $model | ||
| grep ,$model, $WORKSPACE/inductor_log/*/*/*.csv | grep -q ,pass, || ${{ inputs.test_all_subset_models }} |
There was a problem hiding this comment.
This will work only for accuracy. Also for accuracy it is possible that CSV file does not exist (major failure with E2E) in this case the one liner above won't work. I think you need more sophisticated script to handle accuracy/performance and all corner cases. The idea is you need to check that every model from the subset was successfully executed.
scripts/check_inductor_report.py
Outdated
|
|
||
|
|
||
| def check_report(suite, dtype, mode, test_mode, device, models_file): | ||
| inductor_log_dir = Path("torch_compile_debug") / suite / dtype |
There was a problem hiding this comment.
I think the right location is inductor_log. Anyways, it is better to pass it as a parameter.
scripts/check_inductor_report.py
Outdated
| argparser = argparse.ArgumentParser() | ||
| argparser.add_argument("--suite", required=True) | ||
| argparser.add_argument("--dtype", required=True) | ||
| argparser.add_argument("--mode", required=True, choices=("training", "inference")) |
There was a problem hiding this comment.
The mode sting can be inference-no-freezing (see the workflow inputs). The location needs to be inference in this case.
There was a problem hiding this comment.
The location needs to be inference in this case.
What do you mean? Here is example of a CSV file name: timm_models/amp_fp16/inductor_timm_models_amp_fp16_inference-no-freezing_xpu_accuracy.csv
scripts/check_inductor_report.py
Outdated
| exitcode = 0 | ||
|
|
||
| with open(models_file, encoding="utf-8") as f: | ||
| subset = f.read().strip().split("\n") |
There was a problem hiding this comment.
This leaves newlines.
f.read().splitlines() works
For #2246