Zr te doc edits #1745
Open
Zr te doc edits #1745
Conversation
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
The documentation is very clean. I just found a few minor style alignment issues. Signed-off-by: Zenobia "Z" Redeaux <[email protected]>
The text reads well. Just a note that our technical documentation style avoids ableist language (such as "see"), Latin terminology, and the use of "please." Also, if we tell the reader something "should" occur, we need to tell them what to do if it doesn't. Signed-off-by: Zenobia "Z" Redeaux <[email protected]>
Signed-off-by: Zenobia "Z" Redeaux <[email protected]>
Missed two verbs that should be in the past tense in the summary. Signed-off-by: Zenobia "Z" Redeaux <[email protected]>
Try to ensure section Head 1s have some sort of lead-in text. Signed-off-by: Zenobia "Z" Redeaux <[email protected]>
Signed-off-by: Zenobia "Z" Redeaux <[email protected]>
ptrendx
reviewed
May 5, 2025
| "\n", | ||
| "<figure align=\"center\">\n", | ||
| "<img src=\"delayed_scaling.png\" width=\"80%\">\n", | ||
| "<figcaption> Figure 3: Delayed scaling strategy. The FP8 operator uses scaling factor obtained using the history of amaxes (maximums of absolute values) seen in some number of previous iterations and produces both the FP8 output and the current amax, which gets stored in the history.</figcaption>\n", | ||
| "</figure>\n", | ||
| "\n", | ||
| "As one can see in Figure 3, delayed scaling strategy requires both storing the history of amaxes, but also choosing a recipe for converting that history into the scaling factor used in the next iteration." | ||
| "In Figure 3, the delayed scaling strategy requires both storing the history of amaxes but also choosing a recipe for converting that history into the scaling factor used in the next iteration." |
There was a problem hiding this comment.
I'm not sure if the meaning here is exactly the same.
What we wanted to say here is that Figure 3 shows that the delayed scaling strategy requires those things in general. The changed sentence (at least the way I read it) suggests that those things are required only in the context of that figure.
There was a problem hiding this comment.
Maybe it should say "As shown in Figure 3"?
|
|
||
| Installation (development build) | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| .. warning:: | ||
|
|
||
| While the development build of Transformer Engine could contain new features not available in | ||
| the official build yet, it is not supported and so its usage is not recommended for general | ||
| the official build, those features are not supported, so their usage is not recommended for general |
There was a problem hiding this comment.
We meant to say here that the development build of Transformer Engine is not supported, not that those features are not supported.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Style alignment and minor typo edits to Transformer Engine documentation.
Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: