Conversation
iron/common/stream/__init__.py already states the rule -- its submodules need onnx/pyyaml, "so importing an operator must not pull them in" -- and requirements_stream.txt promises the stream operator's test skips itself when stream-dse is absent. Neither holds. iron/operators/__init__.py re-exports SwiGLUPrefillStream, swiglu_prefill_stream/op.py imports ELTWISE_MUL, GEMM and SILU from iron.common.stream.ops, and that module imports onnx and onnxscript at module scope for the exporter registry. So a core install cannot import iron.operators at all. The failure is ModuleNotFoundError at collection, which takes out every operator's test, not just the one operator the optional dependency belongs to. The three names op.py wants are StreamKernel instances -- a stream-dse key, a kernel source and its operand layouts. Nothing about them is ONNX. Move that half to iron/common/stream/kernels.py and leave ops.py the registry it is named for: the private-domain schemas, the translation table, and TORCH_OPS binding a torch ATen op to one of those kernels. ops.py imports the kernels it references, so the one-op-per-entry structure is unchanged; kernel_layouts.py takes its layouts from the new module. A module that does not import onnx cannot regain the dependency by accident, which is the point of splitting rather than deferring the import into the functions that use it. iron/tests/core_install.py pins the contract: a child interpreter with onnx and onnxscript hidden from sys.meta_path must still import iron.operators and iron.common.stream.kernels. It fails before this change and passes after. The ONNX path is unaffected -- TORCH_OPS still resolves three ops and translation_table three entries.
Collaborator
|
@asyms As a sanity check, does this make sense? |
Contributor
Author
|
This work was done before #150 landed, which removed re-export. Some redesign also happened at that time, so right now this is mostly no-op. |
Contributor
CI Test Results4c7bcd1 (2026_09_18_18_17_32) IRON - CI SummaryExamplesiron/applications/llama_3.2_1b
Smalliron/operators/axpy
iron/operators/dequant
iron/operators/elementwise_add
iron/operators/elementwise_mul
iron/operators/flm/gemm
iron/operators/gelu
iron/operators/gemm
iron/operators/gemv
iron/operators/layer_norm
iron/operators/leaky_relu
iron/operators/mem_copy
iron/operators/mha
iron/operators/relu
iron/operators/repeat
iron/operators/rms_norm
iron/operators/rope
iron/operators/sigmoid
iron/operators/silu
iron/operators/softmax
iron/operators/strided_copy
iron/operators/swiglu_decode
iron/operators/swiglu_prefill
iron/operators/tanh
iron/operators/transpose
Krackan - SmallIRONTested on iron/operators/axpy
iron/operators/dequant
iron/operators/elementwise_add
iron/operators/elementwise_mul
iron/operators/flm/gemm
iron/operators/gelu
iron/operators/gemm
iron/operators/gemv
iron/operators/layer_norm
iron/operators/leaky_relu
iron/operators/mem_copy
iron/operators/mha
iron/operators/relu
iron/operators/repeat
iron/operators/rms_norm
iron/operators/rope
iron/operators/sigmoid
iron/operators/silu
iron/operators/softmax
iron/operators/strided_copy
iron/operators/swiglu_decode
iron/operators/swiglu_prefill
iron/operators/tanh
iron/operators/transpose
Krackan - ExamplesIRONTested on iron/applications/llama_3.2_1b
Phoenix - SmallIRONTested on iron/operators/axpy
iron/operators/dequant
iron/operators/elementwise_add
iron/operators/elementwise_mul
iron/operators/flm/gemm
iron/operators/gelu
iron/operators/gemm
iron/operators/gemv
iron/operators/layer_norm
iron/operators/leaky_relu
iron/operators/mem_copy
iron/operators/mha
iron/operators/relu
iron/operators/repeat
iron/operators/rms_norm
iron/operators/rope
iron/operators/sigmoid
iron/operators/silu
iron/operators/softmax
iron/operators/strided_copy
iron/operators/swiglu_decode
iron/operators/swiglu_prefill
iron/operators/tanh
iron/operators/transpose
Phoenix - ExamplesIRONTested on Trend tables omitted, the comment hit GitHub's size limit. Full report in the workflow run. |
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.
iron/common/stream/__init__.pystates the rule -- its submodules needonnx/pyyaml, "so importingan operator must not pull them in" -- and
requirements_stream.txtpromises the stream operator'stest skips itself when stream-dse is absent. Neither holds.
iron/operators/__init__.pyre-exportsSwiGLUPrefillStream;swiglu_prefill_stream/op.pyimportsELTWISE_MUL,GEMMandSILUfromiron.common.stream.ops; and that module importsonnxandonnxscriptat module scope. So a core install cannot importiron.operatorsat all -- aModuleNotFoundErrorat collection, which takes out every operator's tests, not just the one operatorthe optional dependency belongs to.
Added
iron/common/stream/kernels.py, holding theStreamKerneldefinitions and operand layouts.iron/tests/core_install.py, pinning the contract: a child interpreter withonnx/onnxscripthidden fromsys.meta_pathmust still importiron.operatorsandiron.common.stream.kernels.Changed
ops.pykeeps only the registry it is named for -- the private-domain schemas, the translation table, andTORCH_OPS-- and imports the kernels it references.swiglu_prefill_stream/op.pyandiron/tests/stream/kernel_layouts.pytake their names from the new module.Removed
Splitting rather than deferring the import into the functions that use it is deliberate: a module that
does not import
onnxcannot regain the dependency by accident.Verified on
27cf75d:pytest iron/tests/core_install.py-> 20 passed;kernel_layouts.pyskips asdesigned without stream-dse;
TORCH_OPSstill resolves 3 ops andtranslation_table()3 entries;black --checkclean.Note on rebasing: #203 changed the moved block (the
bfp16_mmulMAC-row split and_gemm_artifacts'base_dir->kernels_dirrename).kernels.pycarriesdevel's current version of that block movedverbatim, not the older copy this branch was first written against.
PR Merge Checklist
develcommit and pointing todevel.