-
Notifications
You must be signed in to change notification settings - Fork 50
[QEff. Finetune]: Enabled FT CI tests. #420
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
Conversation
quic-meetkuma
commented
May 21, 2025
- Enabled CI tests for Finetuning.
- Updated Jenkins file to install torch_qaic as it is required during FT tests.
- Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag.
c615ad9
to
80f8cd3
Compare
@@ -25,6 +25,7 @@ pipeline { | |||
pip install junitparser pytest-xdist && | |||
pip install librosa==0.10.2 soundfile==0.13.1 && #packages needed to load example for whisper testing | |||
pip install --extra-index-url https://download.pytorch.org/whl/cpu timm==1.0.14 torchvision==0.19.1+cpu einops==0.8.1 && #packages to load VLMs | |||
pip install /opt/qti-aic/integrations/torch_qaic/py310/torch_qaic-0.1.0-cp310-cp310-linux_x86_64.whl && # For finetuning tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this replace the torch that is getting installed on line 27?
If yes, we should install this package only at the finetuning tests stage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it won't update existing pytorch installation. The torch_qaic is the add-on for qaic backend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did e2e CI run pass post this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But most of qeff users won't have this package installed.
We should reflect the exact env that is going to be used by customers.
I understand it should not affect anything, but why should we deviate from golden standard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did e2e CI run pass post this change?
Yes, it ran.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But most of qeff users won't have this package installed. We should reflect the exact env that is going to be used by customers. I understand it should not affect anything, but why should we deviate from golden standard?
Thanks for pointing it out. We will have subsequent PRs for FT testcases. We will try to take care this in it by having a new stage before FT test cases.
Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
…on of tests for FT. Removed samsum test case from FT. Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
Are we good to merge this code? Anything pending on this? Please reply to @ochougul comments |
@@ -9,7 +9,7 @@ | |||
|
|||
|
|||
def get_preprocessed_samsum(dataset_config, tokenizer, split, context_length=None): | |||
dataset = datasets.load_dataset("Samsung/samsum", split=split, trust_remote_code=True) | |||
dataset = datasets.load_dataset("knkarthick/samsum", split=split, trust_remote_code=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this approved dataset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be removed in #482
@@ -25,6 +25,7 @@ pipeline { | |||
pip install junitparser pytest-xdist && | |||
pip install librosa==0.10.2 soundfile==0.13.1 && #packages needed to load example for whisper testing | |||
pip install --extra-index-url https://download.pytorch.org/whl/cpu timm==1.0.14 torchvision==0.19.1+cpu einops==0.8.1 && #packages to load VLMs | |||
pip install /opt/qti-aic/integrations/torch_qaic/py310/torch_qaic-0.1.0-cp310-cp310-linux_x86_64.whl && # For finetuning tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did e2e CI run pass post this change?
Hi Meet - Lets do following -
Thanks |
Thanks, Hem. |
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]> Signed-off-by: Amit Raj <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]> Signed-off-by: Amit Raj <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]>
- Enabled CI tests for Finetuning. - Updated Jenkins file to install torch_qaic as it is required during FT tests. - Added finetune as a new pytest flag and updated other existing tests not to trigger for this flag. --------- Signed-off-by: meetkuma <[email protected]> Co-authored-by: Meet Patel <[email protected]> Signed-off-by: Dhiraj Kumar Sah <[email protected]>