Repository navigation
fix: make gen_vendor_confs.py the sole writer of the vendor confs - #461
Merged
lvyufeng merged 1 commit intoSep 28, 2026
Merged
Conversation
`FLAGOS_CODEGEN_ALL=1 python3 scripts/codegen/codegen_ops.py` is the full regeneration command scripts/README.md and docs/development/testing.md both prescribe. It ended with two more conf writers, for backends_metax.conf and backends_dcu.conf, built from `op_info` plus a local fallback set (`metax_triton_fallback`, `dcu_triton_fallback`) in the pre-flagos-ai#272 `cuda`/`flagos_python` vocabulary. Both overwrote their conf unconditionally, so the command the docs call a regeneration silently reverted the routing policy those platforms record -- `mm`, `bmm` and `mean.dim` on MetaX, `slice_backward`, `silu_backward` and the `native_batch_norm` pin of flagos-ai#295/flagos-ai#458 on DCU -- and dropped the three ops the generator's op universe carries on top of `op_info`: `matmul`, `matmul_backward` and `scaled_dot_product_attention`. Measured in issue flagos-ai#459. Delete both writers. gen_vendor_confs.py, which CI already enforces with `--check` in .github/workflows/codegen-checks.yml, becomes the only writer of every vendor conf. The run still publishes backend_coverage.py from the FlagGems coverage it discovers and prints the gen_vendor_confs.py command as a follow-up for the case where that coverage moved. Delegating the call instead was measured and rejected: this container's flag_gems discovers 533 of the 639 ops committed in backend_coverage.py, and running the generator here moved 95 routes in backends_metax.conf (flaggems -> cuda) plus a backends_ppu.conf rewrite, neither of them on the CUDA path this script owns. The per-op diagnosis the deleted tables restated moves to BOXING_GAP_NOTES["metax"], beside the DCU entry and next to the routing it explains; the MetaX gap set itself was already read back out of the conf on every run, so nothing but the prose was ever restated. No route moved. All six vendor confs keep their route lines: route-set SHA-256 unchanged for dcu (454 routes), metax (592), ppu (591) and cuda (416); backends_ppu.conf, backends_musa.conf, backends_ascend.conf, backends_gcu.conf and backends_cuda.conf are byte-identical, and the two regenerated confs differ by comments alone (dcu +17 `# Note:` lines, metax +52 comment lines). gen_vendor_confs.py --check is clean before and after the change, and clean twice in a row. tests/unit/test_vendor_conf_single_writer.py pins the three properties at source level -- no other conf named, no local fallback table rebuilt, no generator imported or run -- with no build, no torch import and no hardware. All three fail against the pre-fix codegen_ops.py. Closes flagos-ai#459 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
AI Agent Information
FLAGOS_CODEGEN_ALL=1 python3 scripts/codegen/codegen_ops.py, the regeneration command the project's own docs prescribe, ended with two extra writers that rebuiltbackends_metax.confandbackends_dcu.conffrom a fallback set that had drifted away from the confs. Issue [AI] [BUG][DCU] codegen_ops.py still rewrites backends_dcu.conf and silently reverts the DCU routing policy #459 recorded the measurement; this PR deletes both writers, leavesgen_vendor_confs.pyas the sole owner of the vendor confs, relocates the diagnosis they carried intoBOXING_GAP_NOTES["metax"], and adds a source-level guard test that fails against the pre-fix file.Summary
scripts/codegen/codegen_ops.pyno longer writestorch_fl/configs/backends_metax.confortorch_fl/configs/backends_dcu.conf. Both were written at the end of everyall_cudarun fromop_infoplus a local fallback set in the pre-#272cuda/flagos_pythonvocabulary, and both overwrote their conf unconditionally — so the documented one-command regeneration silently reverted the routing those platforms record and dropped three ops the generator's op universe carries on top ofop_info.scripts/codegen/gen_vendor_confs.py, which CI already enforces with--check, becomes the only writer of every vendor conf; a run whose FlagGems coverage moved now prints the follow-up command instead of running it. No route changed: all six vendor confs keep their route lines and the two regenerated files differ by comments alone.tests/unit/test_vendor_conf_single_writer.pypins the invariant at source level, with no build, no torch import and no hardware.Change Type
Platforms Affected
The change is in the generators and in the conf ownership rule, not in any route: MetaX and DCU's confs are regenerated with added comments and byte-identical route lines, and PPU, CUDA, MUSA, Ascend and GCU's confs are untouched.
Problem Analysis
What was broken/missing?
FLAGOS_CODEGEN_ALL=1 python3 scripts/codegen/codegen_ops.pyis the full regeneration command thatscripts/README.mdanddocs/development/testing.mdboth prescribe. It wrotetorch_fl/configs/backends_cuda.conf(correct: that file is the CUDA boxing table for the wrappers that run generated), and it also wrotebackends_metax.confandbackends_dcu.conf, from:op_info— this run's op set, which is not the op universegen_vendor_confs.pyroutes over. The generator widens that universe fromregister.incwith vendor-only ops read out of a conf (matmul,matmul_backward) plusEXTRA_ROUTED(scaled_dot_product_attention). Those three ops are in the committed confs and were silently deleted by the codegen run.flaggems_forced_cudaplus a per-platform literal (metax_triton_fallback,dcu_triton_fallback) spelled in the pre-ci: align backend routing with platform FlagGems availability #272cuda | flagos_pythonvocabulary, at a time when the confs had already moved on. The MetaX literal in particular named 24 ops of whichmm,mm.out,bmm,bmm.out,mean.dim,add.Tensor,div.Scalar,div.Tensor,_conj,sort,sort.stable,relu,relu_,sum.dim_IntList,embedding_dense_backward,i0,i0.out,reflection_pad2d,soft_margin_loss,special_i0e,special_i1,im2col,zeroandmul_.Tensorwere alreadycudain the committed conf, and 9 of which (conv_transpose2d,scaled_mm,as_strided_copy,prelu,lift_fresh_copy,resolve_conj,t_copy, …) are absent from the conf altogether.The route writer's answer for "not a FlagGems gap" was
flagos_python, which is not what the conf says today: the confs are FlagGems-first and pin the exceptions tocuda.Why did it happen?
The generator reached the confs first and the confs then grew a second, authoritative writer (
gen_vendor_confs.py, enforced by.github/workflows/codegen-checks.yml:62-63). Nobody deleted the first one, so from then on two writers disagreed about the same artifact, and the one that ran on the CUDA path won. The drift was invisible to CI because the damage was never committed — a maintainer running the documented command would see a large diff in the confs and would have reason to read it as expected churn.Investigation process:
scripts/codegen/codegen_ops.pyend to end around the conf writers (theall_cudablock) andscripts/codegen/gen_vendor_confs.py(BOXING_PLATFORMS,VENDORS,route_boxing(),BOXING_GAP_NOTES).BOXING_PLATFORMS's diff branch — the generator readsbackends_metax.confback on every run, so an op FlagGems covers whose conf line readscudais the gap set.route_boxing()checks that set before the C++ route, which is why only 12 ofMETAX_CPP_MEASURED's 17 ops reachflaggems_cppandbmm,bmm.out,sort,sort.stableandsum.dim_IntListstay boxed.514c58dtree (git archive HEAD | tar -x) and ranpython3 scripts/codegen/gen_vendor_confs.py --checkin it: clean, exit 0. The round trip was sound before the fix, which is why the clobber only ever appeared as an uncommitted diff.gen_vendor_confs.pyfromcodegen_ops.py) before choosing the printed hint — see "Key design decisions".git show HEAD:scripts/codegen/codegen_ops.pybefore keeping them.Solution Design
Implementation approach:
codegen_ops.py; the module stops naming any conf butbackends_cuda.conf.render_flaggems_coverage(...)producingscripts/codegen/backend_coverage.py— because that file is the input togen_vendor_confs.pyand codegen_ops.py is the only place the FlagGems Python surface is discovered.BOXING_GAP_NOTES["metax"]ingen_vendor_confs.py, beside the DCU entry that already lives there and next to the routing it explains. The gap set was never restated — it is read back out of the conf — so only the prose moved.gen_vendor_confs.py'sMETAX_CPP_MEASUREDcomment,codegen_ops.py's_conjcomment,tests/integration/ops/test_sum_dispatch.py,tests/integration/ops/test_metax_flaggems.py×3,docs/reference/operator-support.md×3 including the historicalslice.Tensorrow), and correct.claude/skills/pre-pr-checks/SKILL.md, whose conflict procedure told the reader to regeneratetorch_fl/configs/backends_*.confwithcodegen_ops.py— which is the bug this PR removes.Key design decisions:
The follow-up is printed, not run — a deliberate deviation from the issue's first suggestion. Issue #459's Proposed Solution item 1 says "have
codegen_ops.pycallgen_vendor_confs.pyat the end of anall_cudarun", and item 2 offers the printed command as the alternative "if invoking one generator from another is judged too coupled". I implemented item 2 and measured why item 1 is wrong before doing so:codegen_ops.pydiscovers the FlagGems coverage from the installedflag_gems, so the ceiling is environment-dependent. In this container (flag_gems 5.3.4.post1.dev1) it discovers 533 of the 639 ops committed inbackend_coverage.py. A delegated variant of this fix therefore regenerated six vendor confs from that ceiling, not two:backends_metax.confmoved 95 routes (flaggems→cuda) andbackends_ppu.confwas rewritten as well — neither platform is on the CUDA path this script owns, and the diff a reviewer would read is CUDA-binding churn. Every op it moved is a member of both coverage sets, so the movement is not an artifact of the version gap. Stating the follow-up keeps one property that is the whole point of #459 — a full regeneration performs no routing change — while a run that really did move the ceiling shows it right above the hint and in the printed command.Jumping to
cudafirst would invert the generator's decision. The deleted writers answered "not a FlagGems gap" withflagos_python; the shipped confs answer it withcuda, and which one is right is per-platform and recorded in the gap sets. Deleting the writers is what makes that decision single-sourced.The guard is source-level on purpose. The regression is cheap to reintroduce (the shortcut "just write the conf here too" is always available) and expensive to notice (generated-file churn).
tests/unit/test_vendor_conf_single_writer.pyusesastoncodegen_ops.pyand importsgen_vendor_confs.pyby path, so it costs no build, no torch import and no hardware, and it fails at the line that would do the writing.Code changes by file:
scripts/codegen/codegen_ops.py(2629-2633, 3085-3122): removed thebackends_metax.confandbackends_dcu.confwriters and their comment blocks (279 lines net); replaced them with the printed catch-up hint; removed the now-unusedimport subprocess; repointed the_conjcomment atBOXING_GAP_NOTES.scripts/codegen/gen_vendor_confs.py(169-180, 322-395, 1584-1597): rewrote theBOXING_PLATFORMScomment to describe the diff round trip and the single-writer precondition it depends on; added theBOXING_GAP_NOTES["metax"]entry carrying the diagnosis relocated out ofmetax_triton_fallback; repointed theMETAX_CPP_MEASUREDcomment at the gap set and the notes.tests/unit/test_vendor_conf_single_writer.py(new, 253 lines): six tests — the conf-path scan,cuda's absence frombuild_all(), the legacy-set scan, the no-shell-out scan, the printed-command scan, and therender_flaggems_coveragepin.torch_fl/configs/backends_dcu.conf(+17 lines),torch_fl/configs/backends_metax.conf(+52 lines): regenerated; comments only, zero route-line changes.tests/integration/ops/test_sum_dispatch.py(154),tests/integration/ops/test_metax_flaggems.py(42, 78, 262): pointed the MetaXsum.dim_IntListandslice.Tensorexplanations atBOXING_GAP_NOTES["metax"]instead of the deleted literal. Comments and docstrings only.docs/reference/operator-support.md: repointed three prose references, and added a 2026-09-28 Update History row recording the comment-only regeneration, the before/after conf and route-set SHAs, and the fact that no survey was rerun because no route moved..claude/skills/pre-pr-checks/SKILL.md(56-84): split the "regenerate rather than merge" procedure by artifact owner —csrc/aten/generated/*iscodegen_ops.py's,torch_fl/configs/backends_*.confisgen_vendor_confs.py's — and fixed the idempotency check, which compared the worktree against a freshly checked-out index and so reported "NOT idempotent" for any correct run.Changes by commit:
d18914b-fix: make gen_vendor_confs.py the sole writer of the vendor confs: the whole change, as described above.Verification
Pre-submission Checklist
Linting Results
$ ruff check All checks passed! $ ruff format --check 334 files already formattedTest Results
The warning is the pre-existing unregistered
anyplatformmark, not a new one.The three new guards, run against the pre-fix
codegen_ops.py(git show HEAD:...swapped in for the fix):with, verbatim:
The wider unit suite cannot run in this container at the moment, for a reason that is not this change.
import torch_flraisesbecause every
torch_fl/_C*.soon this box predates that pybind setter (strings -a torch_fl/_C.cpython-310-x86_64-linux-gnu.so | grep -c _set_backend_config_path→0, for the copy in this tree, in the main checkout and in every sibling worktree). Every test module that imports the package therefore errors at collection. This is shown to be independent of the change by running the same five files in the pristine514c58dtree and in this branch, back to back; the output is identical, including the collection errors:The five files (
test_flaggems_pointwise_dispatch.py,test_musa_rng_bridge.py,test_startup_subprocess.py,test_transformers_hf_tests.py,test_vendor_libtorch_overlay.py) were also grepped forconfigs/,codegenandbackends_— zero matches — and the change adds no code to any import path they take. An earlier run of the same five files in this container, from before the extension drift, collected 78 tests and failed 11 of them, all of them in the same environment-dependent categories. None of that is claimed as green here: what is claimed is that the three suites above (79 passed) exercise every file this PR changes, and that the failures outside them reproduce on the base commit.Manual Verification
The generator round trip is idempotent and clean, before and after:
A full post-fix regeneration leaves the vendor confs bit-identical. Run in a scratch copy of
514c58dwith this branch'scodegen_ops.pyin place:Before the fix those two SHAs changed on every such run; #459's measurement of one of them is the 2037 → 2034 op regression, 531 moved routes and 121 deleted
# Note:lines, after whichgen_vendor_confs.py --checkfailed on three confs.No route moved anywhere. Route lines and route-set SHA-256 (
sha256of the active-route list) before → after:backends_dcu.conf2c51253253a328f9…unchanged1d0c00e8…→0dd915f8b1377125ae0b2700c4a444489bed80f4156ef9907fa7c80fd9da6198(17# Note:lines added)backends_metax.confb07ca8a02a76986b…unchangedbb1dc5c4…→8af0f654bb27358c93444b5a1674d447d566f6d3c535d591ee80714da7e95a22(52 comment lines added)backends_ppu.confc39b39635122b644…unchangedbackends_cuda.confaacb66bca5dfc08d…unchangedbackends_musa.conf,backends_ascend.conf,backends_gcu.confCode Quality Verification
Style Consistency
Edge Cases Considered
backends_cuda.conf. It is deliberately kept: it is not a vendor conf, it is the CUDA boxing table of the wrappers this run generated, and it is not an input togen_vendor_confs.py(which enumerates fromcsrc/aten/generated/register.inc). The guard pins thatcudais not inbuild_all()'s platform set, so the exemption cannot silently widen.metax_triton_fallback. Six references pointed at the deleted literal; all six now point at the conf andBOXING_GAP_NOTES["metax"], including the operator-support row from 2026-09-16, which keeps its original wording and carries a pointer.import gen_vendor_confs. Rejected and pinned: the module parsessys.argvat import time, andcodegen_ops.pyis imported by the FlagGems register generators, so importing one from the other would consume the caller's arguments. This is why the guards assert the absence of an import as well as of a subprocess call.slice_backwardand DCU'sslice_backwardare pinned for unrelated causes and the relocated notes say so, so the shared name does not read as a shared reason.Potential Risks
Rollback Plan
Revert
d18914b. The change is confined to two generators, two conf files (comments only) and tests, and it moves no runtime route, so a revert restores the previous bytes for every conf.Related Work
native_batch_normpin the deleted writer used to revert)Explicitly Not Included
tests/manual/flaggems_overload_survey.py. No route moved, so there is nothing to re-measure; the report's Update History row records the conf SHAs and says so explicitly rather than implying a survey was taken.backend_coverage.pyor to the FlagGems coverage discovery incodegen_ops.py. The 533-of-639 gap in this container is an environment fact, not a defect this PR fixes, and the coverage file is untouched by the commit.codegen_ops.pyand asserts the confs are unchanged. That is what the unit guards do at source level, without a build; a build-and-run CI job would be a larger change and belongs to the maintainer's call.sum.out-style follow-ups [AI] [BUG][DCU] codegen_ops.py still rewrites backends_dcu.conf and silently reverts the DCU routing policy #459 mentions in passing, if any are real: out of scope here.Human Review Notes
Areas needing special attention:
gen_vendor_confs.pyfirst and the printed command second; this PR takes the second. The measurement behind that is in "Key design decisions" and its blast radius (six confs, 95 MetaX routes, a PPU rewrite) is reproducible by adding thesubprocess.runcall back and diffing the tree.BOXING_GAP_NOTES["metax"]is prose carried across from the deleted literal; the assertions it makes aboutmm/bmm/mean.dim, the scalar group,_conj,sortand the device-guard class come from the deleted comment verbatim, while the survey verdicts it points at live intests/integration/ops/test_metax_flaggems.pyanddocs/reference/operator-support.md. If anything there reads as new, it is a copy error and worth flagging.Questions for reviewer:
codegen_ops.pygain a--refresh-vendor-confsflag that runs the generator explicitly (still not by default)?FLAGOS_CODEGEN_ALL=1run, given the cost?Additional Context
Evidence gaps: no accelerator was used for this change. The confs were regenerated on a host with no DCU, MetaX, PPU, Ascend, GCU or MUSA device, so no platform is revalidated by it, and no row of theirs moves in
docs/reference/operator-support.md. The coverage ceilingcodegen_ops.pydiscovers is environment-dependent (533 of 639 committed ops here), which is the reason the follow-up is printed rather than acted on, and the reason a reviewer should not read the printed number in any log excerpt as the repository's coverage.🤖 Generated with Claude Code