Add a flag to benchmarks script to keep the test report directory#96398
Closed
huydhn wants to merge 5 commits intopytorch:masterfrom
Closed
Add a flag to benchmarks script to keep the test report directory#96398huydhn wants to merge 5 commits intopytorch:masterfrom
huydhn wants to merge 5 commits intopytorch:masterfrom
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/96398
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit cbc4fb2: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
huydhn
changed the title
weiwangmeta
reviewed
Mar 9, 2023
weiwangmeta
reviewed
Mar 9, 2023
Contributor
Author
|
@pytorchbot merge -f 'CI only change. Linter has passed' |
Collaborator
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
cyyever
pushed a commit
to cyyever/pytorch_private
that referenced
this pull request
Mar 12, 2023
…6398) I notice from the Rockset data that there are only `float32` records, while there should be both dtypes there. It turns out that the benchmarks script generated by `runner.py` always removes the output directory by default, so there are only records from `float32` running later left. For example, `rm -rf /var/lib/jenkins/workspace/test/test-reports` appeared twice in the CI log https://ossci-raw-job-status.s3.amazonaws.com/log/11840774308. I'm adding a new flag `--keep-output-dir` to keep the output directory. This is off by default as I'm not sure how this script is used internally, people probably expect to see the output directory cleaned up everytime. ### Testing Not really want to start the 10h jobs just to test this small flag, so I'm triple check the change to make sure that there is no bug Pull Request resolved: pytorch/pytorch#96398 Approved by: https://github.com/weiwangmeta
cyyever
pushed a commit
to cyyever/pytorch_private
that referenced
this pull request
Mar 14, 2023
…6398) I notice from the Rockset data that there are only `float32` records, while there should be both dtypes there. It turns out that the benchmarks script generated by `runner.py` always removes the output directory by default, so there are only records from `float32` running later left. For example, `rm -rf /var/lib/jenkins/workspace/test/test-reports` appeared twice in the CI log https://ossci-raw-job-status.s3.amazonaws.com/log/11840774308. I'm adding a new flag `--keep-output-dir` to keep the output directory. This is off by default as I'm not sure how this script is used internally, people probably expect to see the output directory cleaned up everytime. ### Testing Not really want to start the 10h jobs just to test this small flag, so I'm triple check the change to make sure that there is no bug Pull Request resolved: pytorch/pytorch#96398 Approved by: https://github.com/weiwangmeta
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I notice from the Rockset data that there are only
float32records, while there should be both dtypes there. It turns out that the benchmarks script generated byrunner.pyalways removes the output directory by default, so there are only records fromfloat32running later left.For example,
rm -rf /var/lib/jenkins/workspace/test/test-reportsappeared twice in the CI log https://ossci-raw-job-status.s3.amazonaws.com/log/11840774308.I'm adding a new flag
--keep-output-dirto keep the output directory. This is off by default as I'm not sure how this script is used internally, people probably expect to see the output directory cleaned up everytime.Testing
Not really want to start the 10h jobs just to test this small flag, so I'm triple check the change to make sure that there is no bug
cc @soumith @voznesenskym @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @desertfire