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

[Test] Add transformers test #1175

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

Conversation

RUIJIEZHONG66166
Copy link
Contributor

For validating the upstream of transformers, we plan to add regular test for tracking the status of transformers.

RUIJIEZHONG66166 and others added 9 commits December 17, 2024 05:20
Signed-off-by: Dmitry Rogozhkin <[email protected]>
Signed-off-by: Dmitry Rogozhkin <[email protected]>
These logs will be available on the github actions page.

Signed-off-by: Dmitry Rogozhkin <[email protected]>
-k "not ray" is a workoaround for the hang running tests/trainer.
No need a separate testing.

Signed-off-by: Dmitry Rogozhkin <[email protected]>
Signed-off-by: Dmitry Rogozhkin <[email protected]>
Copy link
Contributor

@dvrogozh dvrogozh left a comment

Choose a reason for hiding this comment

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

@RUIJIEZHONG66166 : Please, see comments. However, I suggest you can just merge in #1177 with all the changes I have suggested. (1177 merges into your dev. branch, not main)

@@ -0,0 +1,60 @@
transformers_test=${1:-backbone}
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's drop this script and associated step for now. Please, save script somewhere since we might need it in the future.

name: Linux Transformers Test

on:
workflow_call:
Copy link
Contributor

Choose a reason for hiding this comment

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

Add on: pull_request. We need verify workflow before merging.

type: string
default: 'v4.47.0'
description: Transformers version
transformers_test:
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this. Let's focus on a single test - backbone.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please, drop transformers_test. We will most likely rework how to setup test scope.

env:
NEOReadDebugKeys: ${{ inputs.driver == 'rolling' && '1' || '0' }}
DisableScratchPages: ${{ inputs.driver == 'rolling' && '1' || '0' }}
pytorch: ${{ github.event_name == 'schedule' && 'nightly' || inputs.pytorch }}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing env.python which is used below

conda remove --all -y -n huggingface_transformers_test || rm -rf $(dirname ${CONDA_EXE})/../envs/huggingface_transformers_test
conda create -n huggingface_transformers_test python=${{ env.python }} cmake ninja -y
source activate huggingface_transformers_test
sudo apt-get install -y espeak-ng
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a conda setup steps - move to separate step.

run: |
source activate huggingface_transformers_test
cd ../transformers
TRANSFORMERS_TEST_DEVICE_SPEC=spec.py python3 -m pytest -rsf --make-reports=tests_py tests/*.py | tee tests_log/tests_py.log
Copy link
Contributor

Choose a reason for hiding this comment

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

Add TRANSFORMERS_TEST_DEVICE_SPEC to env.
Drop tee.

if: ${{ ! cancelled() }}
uses: actions/upload-artifact@v4
with:
name: Torch-XPU-Windows-Log-${{ github.event.pull_request.number || github.sha }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename file (Windows is wrong :))

path: |
${{ github.workspace }}/../transformers/reports
${{ github.workspace }}/../transformers/tests_log
- name: Results Check
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this step for now.

type: string
default: ''
description: Pytorch nightly wheel version
transformers_test:
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this.

if: github.event_name == 'schedule' && github.event.schedule == '0 14 * * 0-4' || ${{ inputs.transformers_test }}
uses: ./.github/workflows/_linux_transformers.yml
with:
transformers_test: ${{ github.event_name == 'schedule' && 'tests_py,tests_benchmark,tests_generation,tests_models,tests_pipelines,tests_trainer,tests_utils,backbone,tests_trainer_not_ray' || inputs.transformers_test }}
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this.

dvrogozh and others added 9 commits December 18, 2024 18:01
Signed-off-by: Dmitry Rogozhkin <[email protected]>
Signed-off-by: Dmitry Rogozhkin <[email protected]>
Signed-off-by: Dmitry Rogozhkin <[email protected]>
Signed-off-by: Dmitry Rogozhkin <[email protected]>
Signed-off-by: Dmitry Rogozhkin <[email protected]>
This PR adds a number of fixes for transformers tests:
- [x] Drop all tests but one (backbone)
- [x] Drop reporting
- [x] Add on PR trigger (limited to modifications to workflow file)
- [x] Use actions/checkout to clone Transformers
- [x] Combine steps to install environment
- [x] Install XPU PyTorch first
- [x] Few other adjustments
type: string
default: 'v4.47.0'
description: Transformers version
transformers_test:
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, drop transformers_test. We will most likely rework how to setup test scope.

Comment on lines 133 to 164
# On-demand launch
- name: OnDemand Test (${transformers_test})
if: github.event_name == 'workflow_dispatch'
run: |
source activate huggingface_transformers_test
cd transformers
# check param
function contains() {
contains_status="echo 'Start $2 ...'"
{
[[ $1 =~ (^|,)$2($|,) ]]
} || {
echo "[Warning] $2 is not suppotted type! Skipped!"
contains_status="continue"
}
}
set -xe
for transformers_test in $(echo ${{ input.transformers_test }} |sed 's/,/ /g')
do
contains "test_py,benchmark,generation,models,pipelines,trainer,utils,backbone,trainer_not_ray" $transformers_test
$contains_status
if [ "${transformers_test}" == "test_py" ];then
python3 -m pytest -rsf --make-reports=tests_py tests/*.py
elif [ "${transformers_test}" == "trainer_not_ray" ];then
python3 -m pytest -rsf --make-reports=tests_trainer_not_ray -k "not ray" tests/trainer
elif [ "${transformers_test}" == "backbone" ];then
python3 -m pytest -rsf --make-reports=tests_backbone -k backbone tests
else
python3 -m pytest -rsf --make-reports=tests_${transformers_test} tests/${transformers_test}
fi
done
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop this. As we agreed - let's do any additional tests on a follow up commit.

Basically limit this change to just s/workflow_call/workflow_dispatch.

@@ -39,6 +39,8 @@ on:
type: string
default: 'v4.47.0'
description: Transformers version
workflow_dispatch:
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't work. You will need to duplicate all inputs from workflow_call

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Never mind. Just check. I have renamed workflow_call.

Copy link
Contributor

@dvrogozh dvrogozh 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!

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.

2 participants