Add --no-patches flag option to get-source.sh + freeze pytest<9 to address failing PyTorch unit tests#456
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an opt-out switch to skip applying WIP GitHub patches during source checkout for the AArch64 TensorFlow and PyTorch build scripts, and documents the new option in each component’s changelog.
Changes:
- Gate TensorFlow patch application in
tensorflow-aarch64/get-source.shbehind a--no-patchesflag. - Gate PyTorch and nested oneDNN patch application in
pytorch-aarch64/get-source.shbehind the same--no-patchesflag. - Update both framework changelogs to note the new
--no-patchesoption.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| ML-Frameworks/tensorflow-aarch64/get-source.sh | Adds --no-patches conditional around TensorFlow patch application. |
| ML-Frameworks/tensorflow-aarch64/CHANGELOG.md | Documents the new TensorFlow --no-patches option. |
| ML-Frameworks/pytorch-aarch64/get-source.sh | Adds --no-patches conditional around PyTorch and oneDNN patch application. |
| ML-Frameworks/pytorch-aarch64/CHANGELOG.md | Documents the new PyTorch/oneDNN --no-patches option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
puneetmatharu
force-pushed
the
add-no-patches-flag-to-get-source-sh
branch
2 times, most recently
from
19df3bb to
c7c9a87
Compare
puneetmatharu
changed the title
--no-patches flag option to get-source.sh--no-patches flag option to get-source.sh + freeze pytest<9 to address failing PyTorch unit tests
|
Probably a good idea to try and get this PR very soon to fix the failing CI. |
|
This is definitely my fault for not fully thinking out this feature request, but I think I envisioned
|
puneetmatharu
force-pushed
the
add-no-patches-flag-to-get-source-sh
branch
3 times, most recently
from
cb4a8bf to
8c39ce7
Compare
|
Done! |
nSircombe
left a comment
nSircombe
left a comment
There was a problem hiding this comment.
hummm. I think my 'assumption' about what --no-patches is differs from @jondea.
I assumed it would just skip he patches. Keeping hashes the same.
I think using the upstream hashes would / should be a separate flag, so getting the behaviour @jondea is shooting for would need --no-patches --original-hashes or similar.
| ### Added | ||
| - TorchAO installation via nightly AArch64 wheel in Dockerfile | ||
| - Adds a `--no-patches` option to `build.sh` and `get-source.sh` to disable patch application and use PyTorch's upstream submodule | ||
| > versions for ideep, oneDNN, and KleidiAI. |
There was a problem hiding this comment.
nit: previously for long lines we've just wrapped and indented to line up with the first character in the line above. See L89 and L90.
There was a problem hiding this comment.
Oh this is odd. I'm not sure how the > character got added... I'll fix it.
puneetmatharu
force-pushed
the
add-no-patches-flag-to-get-source-sh
branch
from
8c39ce7 to
ddf7376
Compare
puneetmatharu
force-pushed
the
add-no-patches-flag-to-get-source-sh
branch
from
ddf7376 to
bbc91ec
Compare
|
Okay @nSircombe @jondea I've opted for a |
nSircombe
left a comment
nSircombe
left a comment
There was a problem hiding this comment.
The PyTorch changes look fine to me. We should use the same flags for TF too though.
No description provided.