Conversation
egparedes
marked this pull request as ready for review
July 30, 2026 17:58
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
July 30, 2026 17:58
55f9fcb to
706dcba
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
July 31, 2026 16:15
706dcba to
605676d
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
August 17, 2026 15:07
605676d to
f120462
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
August 20, 2026 16:17
f120462 to
b70d427
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
August 20, 2026 16:52
b70d427 to
d670473
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
2 times, most recently
from
August 26, 2026 11:05
eff4d3c to
ded2e4d
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
August 28, 2026 13:06
ded2e4d to
33f9c04
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
August 28, 2026 13:09
33f9c04 to
7f833bc
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 1, 2026 18:30
7f833bc to
03ccbeb
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 9, 2026 16:34
03ccbeb to
dc905ed
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 9, 2026 16:51
dc905ed to
308efe8
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 10, 2026 11:54
308efe8 to
b93ea79
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 14, 2026 08:52
b93ea79 to
1d518bb
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 17, 2026 15:21
1d518bb to
2f02b72
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 23, 2026 19:28
2f02b72 to
634c167
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 26, 2026 07:50
634c167 to
0296edb
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
September 30, 2026 06:17
0296edb to
5f01f92
Compare
Contributor
Author
|
Following the review of #2808, its builder-level device check ( @tehrengruber, the same "where do consistency checks end" question applies to these, so this is the place to decide whether they stay: they are the only checks that also cover |
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
October 1, 2026 13:38
5f01f92 to
cd21394
Compare
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
October 1, 2026 14:59
cd21394 to
0151775
Compare
egparedes
added a commit
that referenced
this pull request
Oct 1, 2026
) `factory-boy` is a test-data library, but `gt4py.next` used it in production to compose the GTFN and DaCe backends and their compile workflows. Every object it built is already a frozen dataclass, so the `Trait`/`SubFactory`/`SelfAttribute`/`LazyAttribute` machinery added a second construction language that no type checker can see, and its `__`-path overrides failed silently (`run_gtfn_imperative` was the declarative backend under another name for its whole life). Replace the eight factory classes with plain builder functions and move `factory-boy` to the `test` dependency group (the cartesian and eve IR test-data factories keep using it for its intended purpose). The builders follow three rules (ADR 0028): 1. **Shared settings live in one config.** `GTFNConfig` and `DaCeConfig` (both extending `backend.ToolchainConfig`) hold what the steps must agree on: device, build type, cache lifetime, data layout, translation caching, and for DaCe auto-optimize and the external workspace. Defaults are read from `gt4py.next.config` when the config is created. 2. **Every step is created by a step builder that receives the config** and takes only step-local settings as keyword arguments, so a shared setting cannot be set for one step alone. A step is customized by passing a different builder: a `functools.partial` of the default one, or any callable taking the config. 3. **The toolchain builder owns the composition.** It wraps the translation step in the cache after its builder ran, so a customization always lands on the bare step. A fully custom step builder is responsible for configuring its step consistently with the config it receives; its output is not validated. ```python gtfn.make_gtfn_toolchain( gtfn.GTFNConfig(gpu=True), name_postfix="_no_transforms", translation=functools.partial(gtfn.make_gtfn_translation, enable_itir_transforms=False), ) ``` mypy rejects misspelled step-local settings, wrong value types, and attempts to set a shared setting through a step builder; the same mistakes raise `TypeError` when the toolchain is built. API: - `make_gtfn_toolchain`, `make_gtfn_compile_workflow`, `make_gtfn_translation`, `make_gtfn_bindings`, `make_gtfn_compiler`, `make_gtfn_build_system`. - `make_dace_toolchain`, `make_dace_compile_workflow`, `make_dace_translator`, `make_dace_bindings`, `make_dace_compiler`. - The step builders take step-local settings under the step's own field names, e.g. `make_dace_translator(cfg, auto_optimize_args=..., disable_itir_transforms=..., disable_field_origin_on_program_arguments=...)` and `make_dace_compiler(cfg, add_gpu_trace_markers=...)`. The step-builder types (`GTFNTranslationBuilder`, `GTFNBindingsBuilder`, `GTFNCompilationBuilder` and their `DaCe*` counterparts) are public in `runners.gtfn` and `runners.dace`. - `DaCeTranslator` derives `unit_strides_kind` itself (next to `gpu` and `constant_symbols`) and rejects these three keys in `auto_optimize_args` on construction, so the check covers every way a translator is built, including direct construction and `dataclasses.replace`. - The translation-cache wrapping is shared in `otf.compilation.cache.persistent_translation_cache`. - `make_dace_backend` keeps its flat keyword signature as a front end over `make_dace_toolchain`, but is **deprecated** (`DeprecationWarning`). It builds field-identical toolchains for the same arguments, so external callers (icon4py) keep working until they migrate. - A compile workflow built on its own now caches its translation step by default, like the toolchains; `GTFNConfig(cached_translation=False)` / `DaCeConfig(cached_translation=False)` opt out. - `GTFNTranslationStep.device_type` has no default any more: a builder that forgets to pass it fails instead of silently targeting the CPU. Rebase adaptation (2026-09): while this PR was pending, the gtfn imperative backend was removed from the codebase in #2877, resolving #2810. `run_gtfn_imperative` is therefore dropped instead of fixed, together with the two `#2810` xfails; the incident itself is documented in the Context of ADR 0028. Latent bug: `run_gtfn_no_transforms.name` was `run_gtfn_cpu`, colliding with `run_gtfn`. It is now `run_gtfn_cpu_no_transforms`. This rotates no cache -- the build cache keys on the entry-point name plus a fingerprint of the `ExtensionSource`, and the translation-cache directory is keyed on the literal backend family (`gtfn` / `dace`). `Backend.name` is only used in the metrics metadata, in the per-name bookkeeping of compiled-program pools, and in the DaCe `__sdfg__` check for a `dace` backend (which neither name passes), so the collision's real cost was two distinct backends sharing one metrics identity and one pool name. All other pre-built backends are unchanged, verified field-by-field against the previous construction, except that the DaCe translators no longer store `unit_strides_kind` in `auto_optimize_args` (the translator now derives it, with the same value). This rotates the DaCe translation-cache keys once. Removing the factories also removed the 8 `# type: ignore[assignment] # factory-boy typing not precise enough` suppressions in `src/`, which had been masking real typing problems. One remains as a scoped, documented `type: ignore` in `make_gtfn_bindings`: `OTFCompileWorkflow` is not parameterized over the code spec, while `ExtensionGenerator` accepts only C++-like specs. The pipeline rework (#2743) drops the ignore, but only because its `workflow.Step` fields accept `ProgramSource[Any]`; parameterizing the pipeline over the code spec is left to a follow-up (see ADR 0029). Migration for downstream code: - `GTFNBackendFactory(gpu=on_gpu)` -> `make_gtfn_toolchain(GTFNConfig(gpu=on_gpu))` - `DaCeBackendFactory(..., otf_workflow__bare_translation__async_sdfg_call=False)` -> `make_dace_toolchain(DaCeConfig(...), translation=functools.partial(make_dace_translator, async_sdfg_call=False))` - `make_dace_backend(gpu=..., use_metrics=..., optimization_args=..., use_zero_origin=..., ...)` -> `make_dace_toolchain(DaCeConfig(gpu=...), translation=functools.partial(make_dace_translator, use_metrics=..., auto_optimize_args=..., disable_field_origin_on_program_arguments=..., ...))` See ADR 0028.
…lines The workflow-combinator framework introduced by ADR 0011 had grown to a dozen abstractions to express what is, in the end, function composition. Measured against actual use, only `CachedStep` was a deep module; the rest were shallow wrappers around `Callable[[S], T]`, and the reflection loop in `NamedStepSequence.__call__` was `Any`-typed, defeating the static typing ADR 0011 prized. The named pipelines become plain frozen dataclasses with an explicit, fully typed `__call__`: - `backend.Transforms` keeps its input-dependent step *selection* -- the `match` that used to live in `step_order` now lives in `__call__`, where the order is literally readable -- and the `step_order` method is retained only to raise, so a downstream override fails loudly instead of being silently ignored. - `recipes.OTFCompileWorkflow` becomes `backend.CompilePipeline` and spells out its three steps; `otf.recipes` and `otf.toolchain` are deleted. - Both take over emitting the `stage_hook` added in the previous PR. Names, order, count and artifacts are unchanged; the two instrumentation tests that assert the exact stage sequences pass unmodified, which is the proof. Steps are now plain callables, named by the `workflow.Step[S, T]` alias, and customization stays composition-time via `dataclasses.replace`. Deleted: `Workflow`, `ChainableWorkflowMixin`, `ReplaceEnabledWorkflowMixin`, `NamedStepSequence`, `MultiWorkflow`, `StepSequence`, `make_step`, `.chain`, the three adapters in `otf.toolchain`, and the five `adapted_*_factory` wrappers whose only job was to wrap a function into an adapter. `CachedStep`'s body is unchanged; it loses only the mixin bases, and with them `.replace` and `.chain`. Because steps no longer need to be adapter objects, the seven ffront factories collapse to returning either the bare function or a `CachedStep` around it, and the three per-step callers in `decorator.py` lose their wrap/unwrap dance. What ADR 0011's decisions become: named steps with a visible order are now dataclass fields plus an explicit `__call__`; statically typed composition is checked end-to-end instead of through an `Any`-typed reflection loop; customization at composition time is `dataclasses.replace`; and steps still compose across backends because every existing step already satisfies `Step[S, T]`. No behavior change, and -- unlike the naming PR -- no persistent cache key rotates: fingerprints embed a class's qualified name and fields but never its bases, and neither renamed pipeline is reachable from a persistent cache's fingerprint graph. Breaking, with no compatibility aliases: the deleted combinators and the `otf.recipes` / `otf.toolchain` modules, `.replace()` / `.chain()` on the classes that kept them via the mixins, overriding `Transforms.step_order`, `roundtrip.foast_to_gtir_step` (now a data-only step), and `linter_factory(adapter=...)` (the parameter was accepted and ignored). Claude-Session: https://claude.ai/code/session_01R8zRtFMhdJ8c96XJYCXkRk
…ine and Toolchain `dataclasses.replace` is the documented way to customize a pipeline, so a pipeline can be assembled without going through a builder, and the builder-level `check_device_agreement` no longer sees every route. The assembled objects now check agreement themselves in `__post_init__`, which `dataclasses.replace` re-runs: - `CompilePipeline`: all steps that declare a device (`DeviceConfigurable`, looking through `CachedStep`) must declare the same one; the agreed device is exposed as `CompilePipeline.device_type`. - `Toolchain`: the allocator's `__gt_device_type__` must match the device the backend declares. As with the builder check, these check and never mutate, and only see components that declare a device. No persistent cache key changes: neither class is reachable from a persistent cache's fingerprint graph, and no field was added. Docs: HackTheToolchain.md / WorkflowPatterns.md now state that a replaced step is used as given (a `CachedStep` wrapper is replaced too) and that device agreement is checked only for steps declaring `device_type`. ADR 0029 records the constructor-invariant rule and the not-yet-parameterized code-spec typing of `CompilePipeline`.
egparedes
force-pushed
the
otf-split-4-pipeline
branch
from
October 1, 2026 15:31
0151775 to
a641171
Compare
This branch has not been deployed
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.
The workflow-combinator framework introduced by ADR 0011 had grown to a dozen
abstractions to express what is, in the end, function composition. Measured
against actual use, only
CachedStepwas a deep module; the rest were shallowwrappers around
Callable[[S], T], and the reflection loop inNamedStepSequence.__call__wasAny-typed, defeating the static typingADR 0011 prized.
The named pipelines become plain frozen dataclasses with an explicit, fully
typed
__call__:backend.Transformskeeps its input-dependent step selection -- thematchthat used to live in
step_ordernow lives in__call__, where the order isliterally readable -- and the
step_ordermethod is retained only to raise,so a downstream override fails loudly instead of being silently ignored.
recipes.OTFCompileWorkflowbecomesbackend.CompilePipelineand spells outits three steps;
otf.recipesandotf.toolchainare deleted.stage_hookadded in the previous PR. Names,order, count and artifacts are unchanged; the two instrumentation tests that
assert the exact stage sequences pass unmodified, which is the proof.
Steps are now plain callables, named by the
workflow.Step[S, T]alias, andcustomization stays composition-time via
dataclasses.replace. Deleted:Workflow,ChainableWorkflowMixin,ReplaceEnabledWorkflowMixin,NamedStepSequence,MultiWorkflow,StepSequence,make_step,.chain,the three adapters in
otf.toolchain, and the fiveadapted_*_factorywrappers whose only job was to wrap a function into an adapter.
CachedStep'sbody is unchanged; it loses only the mixin bases, and with them
.replaceand.chain.Because steps no longer need to be adapter objects, the seven ffront factories
collapse to returning either the bare function or a
CachedSteparound it, andthe three per-step callers in
decorator.pylose their wrap/unwrap dance.What ADR 0011's decisions become: named steps with a visible order are now
dataclass fields plus an explicit
__call__; statically typed composition ischecked end-to-end instead of through an
Any-typed reflection loop;customization at composition time is
dataclasses.replace; and steps stillcompose across backends because every existing step already satisfies
Step[S, T].Because
dataclasses.replaceis now the customization route, a pipeline canbe assembled without a builder, so the builders'
check_device_agreementnolonger sees every route.
CompilePipelineandToolchaintherefore checkdevice agreement themselves, in
__post_init__, whichdataclasses.replacere-runs:
CompilePipeline: all steps that declare a device (DeviceConfigurable,looking through
CachedStep) must declare the same one, exposed asCompilePipeline.device_type.Toolchain: the allocator's__gt_device_type__must match the device thebackend declares.
Like the builder check, these only check and never mutate. They only see
components that declare a device, so a custom step without
device_typeisnot checked. HackTheToolchain.md and WorkflowPatterns.md now say so, and also
that a step passed to
dataclasses.replaceis used as given: replacingtranslationalso replaces itsCachedStep, unlike passing a step builder.ADR 0029 records the rule.
The
type: ignoreinmake_gtfn_bindingsfrom #2808 is gone, but not becausethe typing was fixed: the
CompilePipelinefields use unparameterizedProgramSource/ExtensionSource, so mypy acceptsExtensionGeneratorthere,and it would also accept a pipeline mixing C++ and SDFG steps. Parameterizing
CompilePipelineover the code spec only helps once the builders returnparameterized pipelines. That is left to a follow-up, recorded in ADR 0029.
The one behavior change is that a pipeline or toolchain whose components
declare different devices now raises
ValueErrorat construction; thepre-built toolchains are unaffected.
Unlike the naming PR, no persistent cache key rotates: fingerprints embed a
class's qualified name and fields but never its bases, neither pipeline is
reachable from a persistent cache's fingerprint graph, and the device checks
add no fields.
Breaking, with no compatibility aliases: the deleted combinators and the
otf.recipes/otf.toolchainmodules,.replace()/.chain()on theclasses that kept them via the mixins, overriding
Transforms.step_order,roundtrip.foast_to_gtir_step(now a data-only step), andlinter_factory(adapter=...)(the parameter was accepted and ignored).