Skip to content

refactor[next]: replace workflow combinators with explicit typed pipelines - #2743

Open
egparedes wants to merge 2 commits into
otf-split-3-observabilityfrom
otf-split-4-pipeline
Open

egparedes wants to merge 2 commits into
otf-split-3-observabilityfrom
otf-split-4-pipeline

Conversation

@egparedes

@egparedes egparedes commented Jul 30, 2026 •

Copy link
Copy Markdown
Contributor

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].

Because dataclasses.replace is now the customization route, a pipeline can
be assembled without a builder, so the builders' check_device_agreement no
longer sees every route. CompilePipeline and Toolchain therefore check
device 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, exposed as
    CompilePipeline.device_type.
  • Toolchain: the allocator's __gt_device_type__ must match the device the
    backend 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_type is
not checked. HackTheToolchain.md and WorkflowPatterns.md now say so, and also
that a step passed to dataclasses.replace is used as given: replacing
translation also replaces its CachedStep, unlike passing a step builder.
ADR 0029 records the rule.

The type: ignore in make_gtfn_bindings from #2808 is gone, but not because
the typing was fixed: the CompilePipeline fields use unparameterized
ProgramSource / ExtensionSource, so mypy accepts ExtensionGenerator there,
and it would also accept a pipeline mixing C++ and SDFG steps. Parameterizing
CompilePipeline over the code spec only helps once the builders return
parameterized 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 ValueError at construction; the
pre-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.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).

@egparedes
egparedes marked this pull request as ready for review July 30, 2026 17:58
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 55f9fcb to 706dcba Compare July 30, 2026 17:58
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 706dcba to 605676d Compare July 31, 2026 16:15
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 605676d to f120462 Compare August 17, 2026 15:07
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from f120462 to b70d427 Compare August 20, 2026 16:17
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from b70d427 to d670473 Compare August 20, 2026 16:52
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch 2 times, most recently from eff4d3c to ded2e4d Compare August 26, 2026 11:05
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from ded2e4d to 33f9c04 Compare August 28, 2026 13:06
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 33f9c04 to 7f833bc Compare August 28, 2026 13:09
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 7f833bc to 03ccbeb Compare September 1, 2026 18:30
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 03ccbeb to dc905ed Compare September 9, 2026 16:34
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from dc905ed to 308efe8 Compare September 9, 2026 16:51
@egparedes

Copy link
Copy Markdown
Contributor Author

Following the review of #2808, its builder-level device check (check_device_agreement) was removed: default and partially customized steps are in sync by construction, and a fully custom step builder is the caller's responsibility. The construction-time checks in this PR (CompilePipeline.__post_init__ and Toolchain.__post_init__) now define the DeviceConfigurable protocol themselves in backend.py.

@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 dataclasses.replace and direct construction, and removing them is a self-contained change to this PR.

@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from 5f01f92 to cd21394 Compare October 1, 2026 13:38
@egparedes
egparedes force-pushed the otf-split-4-pipeline branch from cd21394 to 0151775 Compare October 1, 2026 14:59
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
egparedes force-pushed the otf-split-4-pipeline branch from 0151775 to a641171 Compare October 1, 2026 15:31

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant