Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a spelling/typo cleanup sweep across MIGraphX source, tests, docs, and build/support files to improve readability and reduce minor friction in comments/messages.
Changes:
- Corrects spelling in comments and documentation across multiple components (ONNX parsing, RNN rewrite, GPU kernels, etc.).
- Fixes typos in user-facing/build-facing text (e.g., CMake fatal error message, driver error string).
- Updates changelog wording and various developer docs/test comments for consistency.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ref_dev_examples.cpp | Fixes comment typos in a reference-device example test. |
| test/py/onnx_backend_test.py | Fixes a typo in a disabled-tests comment. |
| src/targets/gpu/kernels/include/migraphx/kernels/float8_impl.hpp | Corrects spelling in float8 conversion comments (GPU kernel header). |
| src/targets/gpu/compile_gen.cpp | Fixes typo in a codegen comment. |
| src/simplify_qdq.cpp | Fixes typo in quantization-related comment. |
| src/rewrite_rnn.cpp | Fixes typos in GRU/LSTM activation-function comments. |
| src/onnx/parse_softmaxcrossentropyloss.cpp | Fixes typos in comments describing index selection/calculation. |
| src/onnx/parse_attention.cpp | Fixes typo in a raw-mask behavior comment. |
| src/memory_coloring.cpp | Fixes typo in a segment-insertion comment. |
| src/include/migraphx/op/scatter_op.hpp | Fixes typo in scatter op comment. |
| src/include/migraphx/op/onehot.hpp | Fixes typos in OneHot operator documentation. |
| src/include/migraphx/module.hpp | Fixes typo in public API doc comment. |
| src/include/migraphx/float8_impl.hpp | Corrects spelling in float8 conversion comments (host header). |
| src/include/migraphx/common.hpp | Fixes Doxygen tag formatting (@brief). |
| src/driver/main.cpp | Fixes typo in an exception message. |
| docs/dev/onnx_operators.rst | Fixes typos in the operator support matrix table text. |
| docs/dev/dev_intro.rst | Fixes comment typos in developer intro snippet. |
| docs/dev/contributing-to-migraphx.rst | Fixes comment typos in contributing guide snippet. |
| Dockerfile | Fixes typo in a build comment. |
| CMakeLists.txt | Fixes typo in a fatal configuration error message. |
| CHANGELOG.md | Corrects multiple spelling issues in changelog entries/headers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/memory_coloring.cpp
Outdated
| assert(n > 0); | ||
| std::size_t start = 0; | ||
| // Insert at end if it cant fit at the begining | ||
| // Insert at end if it cant fit at the beginning |
There was a problem hiding this comment.
The comment still uses "cant" (without apostrophe) in the sense of "cannot". Elsewhere in the codebase comments use "can't", so this looks like a remaining typo; consider updating to "can't" for consistency.
Suggested change
| // Insert at end if it cant fit at the beginning | |
| // Insert at end if it can't fit at the beginning |
Comment on lines
456
to
459
| weights = info.add_instruction(migraphx::make_op("transpose", {{"permutation", perm}}), | ||
| weights); | ||
| weights = info.add_instruction(migraphx::make_op("gathernd"), weights, gathernd_indicies); | ||
|
|
There was a problem hiding this comment.
The local identifier "gathernd_indicies" is misspelled ("indices"). Since this PR is already cleaning up spelling, consider renaming this variable (and related locals like "batch_dim_indicies") to "*_indices" to improve readability and grep-ability.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4692 +/- ##
========================================
Coverage 92.27% 92.27%
========================================
Files 578 578
Lines 28541 28541
========================================
Hits 26335 26335
Misses 2206 2206
🚀 New features to boost your workflow:
|
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
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.
Motivation
May not be a comprehensive list, but a quick sweep through the codebase for common spelling mistakes.
Technical Details
Changelog Category
Add a
CHANGELOG.mdentry for any option other thanNot Applicable