Skip to content

[Docs] Update docs for the single nested config layout - #5085

Merged
Boss2002n merged 2 commits into
mainfrom
satya/update-config-docs-nested-layout
Sep 2, 2026
Merged

Boss2002n merged 2 commits into
mainfrom
satya/update-config-docs-nested-layout

Conversation

@Boss2002n

Copy link
Copy Markdown
Contributor

No description provided.

The config tree is now one nested layout, `configs/<arch>/<backend>/<op>/
<d_type>/`, resolved by a deterministic path builder; the flat arch-prefixed
directories and every fallback that reached them are gone. Six docs still
described the mid-migration world.

- configs/CLAUDE.md: rewritten as the rulebook for the layout that exists.
  One layout section, the `resolve_config_dir()` / `load_config_json()`
  contract (validation, fail-closed assertions, caching incl. negatives,
  shared-object return), the layered loader-module table, and a section per
  family: GEMM order-of-operations and file contents, the MOE dispatch key
  schemes per backend, the conv four-tier walk, MHC's C-threshold discovery
  and documented gfx942 fallback, and the pinned-tile loader. Adding a config
  and seeding a new arch replace the migration playbook. The planned
  `get_moe_config()` design section is dropped -- `get_moe_dispatch()` shipped.
- README.md: config sections rewritten to match; `utils/core.py` references
  point at `utils/config_utils.py`; MOE naming row and key schemes corrected.
- .github/instructions: placement rules restated for one layout, plus new
  review rules for MOE dispatch tables, kernel-level `backend` defaults, the
  layered `utils/` split, and arch seeding.
- conv/DESIGN.md, conv/README.md: seven flat `configs/conv/{arch}-CONV-*.json`
  references updated to the nested paths, and section 9 now names the shared
  resolver.
- tunning/README.md: one copy destination instead of a migrated-vs-legacy
  split.

Corrections found while checking every claim against the tree: the `kpack`
rule is scoped to gfx950 (the RDNA trees do still carry it), the MHC
C-threshold glob unions the running arch with gfx942, the six-bucket MOE rule
is guidance for newly tuned shapes rather than a description of current
coverage, and the AOT-guard note now matches what each of the two paths
actually does.
@Boss2002n
Boss2002n requested review from a team and brunomazzottiamd and a lite review from Copilot August 28, 2026 15:39
@github-actions github-actions Bot changed the title [Triton] Update docs for the single nested config layout [Docs] Update docs for the single nested config layout Aug 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:gfx1250-ffm-triton Run the five-shard gfx1250 FFM Triton test suite
ci:triton-300x Run an additional Triton test job on MI300X in PRs; main branch always runs both MI35X and MI300X
multigpu Aiter multi-GPU tests on the 8-GPU runner
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: GPT-OSS-120B, DeepSeek-R1-0528, Kimi-K2.5
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 5085 --add-label <label>

PR title tags:
Component tags ([Triton/Gluon], [HIP], [CK], [ASM], ...) are added to the PR title automatically from the changed files and re-synced on every push — change-type tags like [fix]/[Perf] and op tags like [MLA] are left untouched. Add the no-auto-title label to opt this PR out of title tagging.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates AITER Triton documentation to describe a single nested tuned-config layout (configs/<arch>/<backend>/<op>/<d_type>/) and removes remaining references to the legacy flat config tree.

Changes:

  • Updates tuning workflow docs to instruct copying tuned JSON into the nested config tree.
  • Revises top-level Triton docs and config rulebook (configs/CLAUDE.md) to describe the nested layout and loader expectations.
  • Refreshes conv docs to reference nested config locations.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
aiter/ops/triton/utils/_triton/tunning/README.md Updates manual tuning “verify performance” copy-path guidance for the nested layout.
aiter/ops/triton/README.md Rewrites tuned-config documentation to describe the single nested layout and loader behavior.
aiter/ops/triton/conv/README.md Updates conv tuning documentation to the nested config tree paths.
aiter/ops/triton/conv/DESIGN.md Updates conv design docs to reference nested config resolution and probing.
aiter/ops/triton/configs/CLAUDE.md Updates the config “rulebook” to reflect the single nested layout and loader conventions.
.github/instructions/aiter-ops-triton.instructions.md Updates repository review rules to match the new config layout guidance.
Suppressed comments (5)

aiter/ops/triton/README.md:100

  • This description says resolve_config_dir "builds" the path with no probing/candidate list, but the current implementation probes the filesystem for DEFAULT.json and (when backend is None) tries triton then gluon. Either the docs need to match that behavior, or the resolver needs to be updated to match these claims.
It **builds** the path — no probing, no candidate list, no cross-backend or
cross-arch search — and validates every argument against a whitelist, so a bad
value fails closed with an `AssertionError` rather than resolving to an
escaped or wrong directory. `backend` is declared by the caller (gluon kernels
and gluon dispatch paths pass `"gluon"`; everything else takes the `"triton"`

aiter/ops/triton/configs/CLAUDE.md:37

  • This table points at config_utils._dtype_dir(), but there is no utils/config_utils.py in the current tree; the helper is utils/gemm_config_utils.py::_dtype_dir().
| `<d_type>`  | `config_name.lower().replace("-", "_")` — `GEMM-AFP4WFP4` → `gemm_afp4wfp4`. The transform is `config_utils._dtype_dir()` |

aiter/ops/triton/configs/CLAUDE.md:73

  • This describes resolve_config_dir as a deterministic path builder with no probing/fallback chain, but the current implementation explicitly probes the filesystem and has a triton→gluon fallback when backend is omitted (backend=None).
directory; it never probes, never searches, and has no fallback chain:

{AITER_TRITON_CONFIGS_PATH}/{arch}/{backend}/{op}/{_dtype_dir(config_name)}

aiter/ops/triton/README.md:94

  • utils/config_utils.py and the shown resolve_config_dir(...)->str signature do not exist in the current codebase. Today resolve_config_dir lives in utils/gemm_config_utils.py and returns (cfg_dir, name_prefix).
All of it is built by one function in `utils/config_utils.py`:

```python
resolve_config_dir(op, config_name, backend="triton", arch=None) -> str
**aiter/ops/triton/configs/CLAUDE.md:68**
* The `resolve_config_dir` signature/module here doesn't match the current implementation: it lives in `utils/gemm_config_utils.py`, returns `(cfg_dir, name_prefix)`, and probes for `DEFAULT.json` (with `backend=None` trying triton then gluon).
resolve_config_dir(op, config_name, backend="triton", arch=None) -> str

in utils/config_utils.py is the single path builder. It builds the

</details>



---

💡 <a href="/ROCm/aiter/new/main?filename=.github/skills/code-review/SKILL.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add a `code-review` agent skill</a> or configure MCP servers for context-aware, tailored reviews. <a href="https://docs.github.com/en/copilot/how-tos/use-copilot-agents/request-a-code-review/use-code-review#mcp-servers-and-agent-skills" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn more in the docs.</a>

Comment thread aiter/ops/triton/utils/_triton/tunning/README.md
Comment thread aiter/ops/triton/utils/_triton/tunning/README.md
Comment thread aiter/ops/triton/README.md
Comment thread aiter/ops/triton/configs/CLAUDE.md
Comment thread aiter/ops/triton/conv/DESIGN.md
Comment thread .github/instructions/aiter-ops-triton.instructions.md
Copilot AI review requested due to automatic review settings August 31, 2026 06:18
@github-actions github-actions Bot added the Docs label Aug 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

aiter/ops/triton/utils/_triton/tunning/README.md:63

  • "tunned" is a misspelling, and the sentence is ungrammatical ("files ... is"). Update to "tuned" and adjust agreement so the README reads cleanly.
To verify that your tunned JSON config files actually is performant and can be correctly picked up by AITER, first you have to copy the generated JSON config files into the config tree. Every family lives in one nested layout, `configs/<arch>/<backend>/<op>/<d_type>/` (`<path_to_aiter_root>/aiter/ops/triton/configs/CLAUDE.md` is the authoritative rulebook). Files there carry **no arch prefix** — the arch is the directory — and the default file is named exactly `DEFAULT.json`, so drop the arch prefix when copying:

Comment thread aiter/ops/triton/configs/CLAUDE.md

@brunomazzottiamd brunomazzottiamd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Boss2002n
Boss2002n merged commit 90e91d5 into main Sep 2, 2026
28 checks passed
@Boss2002n
Boss2002n deleted the satya/update-config-docs-nested-layout branch September 2, 2026 18:51
brunomazzottiamd pushed a commit that referenced this pull request Sep 8, 2026
…DNA3) and gfx1150 (RDNA3.5) (#5246)

* [Triton] [Conv2D] Add Conv2D configuration files for gfx1101 and gfx1150

Nine tables per architecture in the nested layout, tuned on hardware. Both
architectures currently have none, so every Conv2D family is a hard
FileNotFoundError from load_config_json(required=True) rather than a fallback.

gfx1101 (RDNA3, RX 7800 XT): 190 entries, 20 tuned on the card, 170 inherited
byte-identical from gfx1100.
gfx1150 (RDNA3.5, Radeon 890M): 186 entries, 100 tuned, 86 inherited from
gfx1151.

Entry counts cover every section - shapes, variant pins, M_LEQ_* buckets and
any - not just shapes.

The structure of every file matches its donor exactly: same sections, same key
set per section, same route_exact_only. Routing behaviour is therefore unchanged
and only the values differ. In particular CONV-3X3-NCHW ships only the shape
keys the donor already has, because under route_exact_only the key set is what
_nchw_direct_is_profitable uses to choose between DIRECT_NCHW_3X3 and
CBLOCKED_NCHW, and that comparison was not part of these measurements.

Every candidate was checked for correctness before being timed - rel_l2 < 1e-2
against torch.nn.functional.conv2d, the bar op_tests/triton_tests/conv uses -
and each family was driven through its own public entry point rather than
through the router.

Follows the invitation in
#4869 (comment)

Signed-off-by: Martin Domanský <ragua@email.cz>

* [Triton] [Conv2D] Add gfx1101 and gfx1150 to the conv test SUPPORTED_ARCHS

Without this the two new tables are never exercised by op_tests/triton_tests/conv.

Note that the repository's CI has no RDNA machine, so this does not by itself
cause the tables to be tested anywhere - it only makes the tests runnable on
hardware that has them.

Signed-off-by: Martin Domanský <ragua@email.cz>

* [Triton] Add the two new arches to the config layout table

configs/CLAUDE.md documents which architectures have a config tree. #5085
rewrote that document for the nested layout, so it is maintained and a new arch
belongs in it.

Separate commit from the data on purpose: it is your document, and a doc change
should be reviewable on its own rather than folded into eighteen JSON files.

Signed-off-by: Martin Domanský <ragua@email.cz>

* [Triton] [Conv2D] Drop CONV-3X3-NCHW from this PR

That table is the optional ninth family and adding it is a routing change on
these two architectures, which the rest of this PR is not.

_nchw_direct_is_profitable returns False when has_conv_config("CONV-3X3-NCHW")
is False, so today every 3x3 shape on gfx1101 and gfx1150 takes CBLOCKED_NCHW.
Shipping the table with route_exact_only moves 7 shapes on gfx1101 and 4 on
gfx1150 onto DIRECT_NCHW_3X3, and none of those 11 has a direct-against-cblocked
measurement on either card.

The tables had already been trimmed to the donor's key set so they would carry no
routing change relative to gfx1100 and gfx1151. That was the wrong baseline: the
comparison that matters is against what these cards do now, which is no table at
all.

gfx1200, gfx1250, gfx942 and gfx950 ship no CONV-3X3-NCHW either, so the
remaining eight families are the mandatory set and this PR now carries values
only.

Counts after this commit:

  gfx1101   182 entries, 16 tuned on the card, 166 inherited from gfx1100
  gfx1150   181 entries, 97 tuned on the card,  84 inherited from gfx1151

The table returns in a follow-up that measures both routes per key.

Signed-off-by: Martin Domanský <ragua@email.cz>

---------

Signed-off-by: Martin Domanský <ragua@email.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants