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

[wav2vec2] Remove tensor.item and dynamic slicing operations in the loop that cause graph break #1508

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chaojun-zhang
Copy link

What does this PR do?

Remove tensor.item and dynamic slicing operations in the loop that cause graph break

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

…loop that cause graph break

Change-Id: Ie6b60e72e29c5d1ebc7220e26a106a25b7e41d6e
@chaojun-zhang chaojun-zhang changed the title Remove tensor.item and dynamic slicing operations in the loop that cause graph break [wav2vec2] Remove tensor.item and dynamic slicing operations in the loop that cause graph break Nov 21, 2024
@astachowiczhabana
Copy link
Contributor

@libinta this commit is required with next OH release

@imangohari1
Copy link
Contributor

imangohari1 commented Nov 22, 2024

@chaojun-zhang CC: @astachowiczhabana
Can you please run the following tests with and without these changes so we can make sure these are not introducing regressions? Thanks.

GAUDI2_CI=1 RUN_SLOW=true python -m pytest tests/transformers/tests/models/wav2vec2/ -s -v
GAUDI2_CI=1 RUN_SLOW=true python -m  pytest tests/test_examples.py -v -s -k  test_run_audio_classification_wav2vec2-base_multi_card
GAUDI2_CI=1 RUN_SLOW=true python -m pytest tests/test_examples.py -v -s -k test_run_speech_recognition_ctc_wav2vec2-large-lv60_multi_card

Copy link
Contributor

@imangohari1 imangohari1 left a comment

Choose a reason for hiding this comment

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

@chaojun-zhang CC: @astachowiczhabana Can you please run the following tests with and without these changes so we can make sure these are not introducing regressions? Thanks.

GAUDI2_CI=1 RUN_SLOW=true python -m pytest tests/transformers/tests/models/wav2vec2/ -s -v
GAUDI2_CI=1 RUN_SLOW=true python -m  pytest tests/test_examples.py -v -s -k  test_run_audio_classification_wav2vec2-base_multi_card
GAUDI2_CI=1 RUN_SLOW=true python -m pytest tests/test_examples.py -v -s -k test_run_speech_recognition_ctc_wav2vec2-large-lv60_multi_card

@chaojun-zhang
I ran these tests and one of them is failing compared to main.
GAUDI2_CI=1 RUN_SLOW=true python -m pytest tests/test_examples.py -v -s -k test_run_audio_classification_wav2vec2-base_multi_card
with current changes shows

E   eval_accuracy: 0.7394701086956522 vs 0.715572
E   train_runtime: 201.5022 vs 66.578295
E   train_samples_per_second: 703.581 vs 2827.0517999999997
E   eval_samples_per_second: 308.74 vs 3458.01995

and without shows:

E   eval_accuracy: 0.7338654891304348 vs 0.715572
E   train_runtime: 79.2719 vs 66.578295
E   train_samples_per_second: 2731.957 vs 2827.0517999999997
E   eval_samples_per_second: 2812.678 vs 3458.01995

We need to understand what is casuing the perf desegregation on this.

@jiminha
Copy link
Collaborator

jiminha commented Nov 26, 2024

@astachowiczhabana Could you also check above?

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.

4 participants