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

Save other CI jobs' result (torch/tf pipeline, example, deepspeed etc) #30699

Merged
merged 9 commits into from May 13, 2024

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented May 7, 2024

What does this PR do?

Currently we only save the model results. Let's save all of them.
I still need to work with the quantization job - as it is another separate notification file (and maybe also doctest - less priority for now).

I have to change the single artifact (ci_results) to several one (ci_results_{job_name}) as we can't upload to the same name multiple times.

Screenshot 2024-05-07 181840

See

https://github.com/huggingface/transformers/actions/runs/8988511794

@ydshieh
Copy link
Collaborator Author

ydshieh commented May 7, 2024

I promise this is the last pr I am opening today 😆

@ydshieh
Copy link
Collaborator Author

ydshieh commented May 7, 2024

well, this might need some extra work. No need to approve for now ...sorry being a bit too fast

@ydshieh ydshieh marked this pull request as draft May 7, 2024 15:07
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Comment on lines +65 to +66
name: ci_results_${{ inputs.job }}
path: ci_results_${{ inputs.job }}
Copy link
Collaborator Author

@ydshieh ydshieh May 7, 2024

Choose a reason for hiding this comment

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

I tried not to change this again, but there doesn't seem a better solution.

The problem is that we can't upload the different artifacts to the same name (workflow run will fail).

@ydshieh ydshieh marked this pull request as ready for review May 7, 2024 16:25
Comment on lines +1151 to +1162
# Must have the same keys as in `additional_results`.
# The values are used as the file names where to save the corresponding CI job results.
test_to_result_name = {
"PyTorch pipelines": "torch_pipeline",
"TensorFlow pipelines": "tf_pipeline",
"Examples directory": "example",
"Torch CUDA extension tests": "deepspeed",
}
for job, job_result in additional_results.items():
with open(f"ci_results_{job_name}/{test_to_result_name[job]}_results.json", "w", encoding="UTF-8") as fp:
json.dump(job_result, fp, indent=4, ensure_ascii=False)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the main changes in this PR.

Comment on lines +245 to +250
job_name = os.getenv("CI_TEST_JOB")
if not os.path.isdir(os.path.join(os.getcwd(), f"ci_results_{job_name}")):
os.makedirs(os.path.join(os.getcwd(), f"ci_results_{job_name}"))

with open(f"ci_results_{job_name}/quantization_results.json", "w", encoding="UTF-8") as fp:
json.dump(quantization_results, fp, indent=4, ensure_ascii=False)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Same as above but for quantization

@@ -416,7 +416,7 @@ def per_model_sum(model_category_dict):
reports=sorted_model_reports,
to_truncate=False,
)
file_path = os.path.join(os.getcwd(), "ci_results/model_failures_report.txt")
file_path = os.path.join(os.getcwd(), f"ci_results_{job_name}/model_failures_report.txt")
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(all same changes below too) the same reason as above

Comment on lines +87 to +94

# Upload complete failure tables, as they might be big and only truncated versions could be sent to Slack.
- name: Failure table artifacts
if: ${{ inputs.job == 'run_quantization_torch_gpu' }}
uses: actions/upload-artifact@v4
with:
name: ci_results_${{ inputs.job }}
path: ci_results_${{ inputs.job }}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For quantization and only for it

@ydshieh ydshieh requested a review from amyeroberts May 7, 2024 18:40
Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks!

utils/notification_service.py Show resolved Hide resolved
utils/notification_service.py Outdated Show resolved Hide resolved
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
@ydshieh ydshieh merged commit 82c1625 into main May 13, 2024
8 checks passed
@ydshieh ydshieh deleted the upload_artifacts_3 branch May 13, 2024 15:27
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.

None yet

3 participants