From 93b1cd70ed741bcd60fc7c3af3383ff2438589b9 Mon Sep 17 00:00:00 2001 From: Mustafa Eyceoz Date: Mon, 7 Oct 2024 15:13:09 -0400 Subject: [PATCH] Apply review feedback Signed-off-by: Mustafa Eyceoz --- .spellcheck-en-custom.txt | 1 - docs/training/training-accelerate-dep.md | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.spellcheck-en-custom.txt b/.spellcheck-en-custom.txt index 0507a4d9..8921edf4 100644 --- a/.spellcheck-en-custom.txt +++ b/.spellcheck-en-custom.txt @@ -4,7 +4,6 @@ Abhishek Akash AMDGPU Anil -annunciates arge args arXiv diff --git a/docs/training/training-accelerate-dep.md b/docs/training/training-accelerate-dep.md index 9f78fcc6..53acd251 100644 --- a/docs/training/training-accelerate-dep.md +++ b/docs/training/training-accelerate-dep.md @@ -89,7 +89,7 @@ The inclusion of Accelerate in this manner drastically simplifies the process of ### Performance -There has been no noticeable performance impact observed during development, but we will defer to the Performance and Scale team for final measurements. +There has been no noticeable performance impact observed during development, but we will defer to the final measurements seen from comprehensive performance testing and benchmarking. ### Usability @@ -120,4 +120,4 @@ For example, if we wanted to change how model saving worked, or a user wanted to This expands well into the wider topic of dependency management. A package like Accelerate will have frequent updates and changes moving forward, and we will need to ensure consistent compatibility between our training library releases and the required Accelerate versions. A new dependency also means shipping with another package, with a large set of code. While in most cases this is harmless, a package that is used directly on top of our work in place of our existing code, adds a significant amount of bloat for developers and users alike. The requirement shifts from understanding a line of code, to understanding a full code path behind a line of code, the quality of which cannot be guaranteed by our engineers. -Ultimately, these risks and overhead management costs make sense for Accelerate, because the package provides significant benefit that far outweigh our concerns. When considering additional HF packages, however, there is currently **no obvious benefit** to their inclusion, which only further annunciates the risks associated. It must be made clear that the risks of Accelerate are non-negligible, and that the conveniences and positive impact provided are what push it over the line of inclusion. It is by no means a "no brainer" to further include any additional HF packages at this time, unless they afford us a similar benefit. +Ultimately, these risks and overhead management costs make sense for Accelerate, because the package provides significant benefit that far outweigh our concerns. When considering additional HF packages, however, there is currently **no obvious benefit** to their inclusion, which only further enunciates the risks associated. It must be made clear that the risks of Accelerate are non-negligible, and that the conveniences and positive impact provided are what push it over the line of inclusion. It is by no means a "no brainer" to further include any additional HF packages at this time, unless they afford us a similar benefit.