refactor[next]: drop factory-boy for plain builders + sub-component injection#2669
Draft
egparedes wants to merge 3 commits into
Draft
refactor[next]: drop factory-boy for plain builders + sub-component injection#2669egparedes wants to merge 3 commits into
egparedes wants to merge 3 commits into
Conversation
factory-boy was repurposed as a production composition mechanism for the
GTFN and DaCe backends and their OTF compilation workflows. Every model is
already a plain frozen dataclass, so the Trait/SubFactory/SelfAttribute/
LazyAttribute machinery and the stringly-typed `__`-path overrides added
indirection without value, and carried `type: ignore`s plus a note that
factory-boy is broken.
This replaces all production `factory.Factory` classes with plain builder
functions or direct dataclass construction:
- Leaf steps (Compiler, GTFNTranslationStep, DaCeTranslator, DaCeCompiler)
are now constructed directly.
- GTFN: `make_gtfn_workflow(...)` / `make_gtfn_backend(...)`; Traits became
bool kwargs (gpu, cached) and `__`-paths became real parameters.
- DaCe: `make_dace_workflow(...)`; `DaCeBackendFactory` folded into the
existing `make_dace_backend(...)` (public signature unchanged).
All public pre-built backends keep identical names and wiring
(run_gtfn_*, run_dace_*, gtfn_cpu/gtfn_gpu). The few tests that called the
production factories were migrated to the new builders. factory-boy moves
from runtime `dependencies` to the `test` group (still used by the
cartesian/eve IR test-data factories). Advanced docs updated accordingly.
Aligned with the layered-architecture refactoring proposal ("plain-code
backend builders, not factory-boy"); the Backend->toolchain rename and the
pipeline collapse are intentionally left for later.
…nfig
Follow-up to the factory-boy removal. Flattening the old `__`-path overrides
into explicit maker kwargs re-coupled each maker to its sub-components'
internals (most visibly `make_dace_backend`, forwarding ~6 translator knobs).
Replace that with lightweight dependency injection: a maker exposes only
cross-cutting configuration (device, caching, auto-optimize, build type) and
accepts pre-built sub-components. A single-component knob is set by injecting
that component, e.g.
make_gtfn_backend(translation=GTFNTranslationStep(enable_itir_transforms=False))
Details:
- `workflow.with_fields(step, **fields)`: guarded helper that stamps
cross-cutting fields (device_type; dace auto_optimize) onto a step only when
it is a dataclass declaring them, so injecting an arbitrary callable is safe.
- GTFN makers drop `enable_itir_transforms`/`use_imperative_backend`, gain
`translation`/`bindings`/`compilation`/`decoration` injection params.
- `DaCeTranslator` gets defaults for its formerly-required fields; a new
`make_dace_translator(...)` owns the translator-local knobs and the relocated
`optimization_args` validation / `unit_strides_kind` derivation.
- `make_dace_workflow`/`make_dace_backend` shed their translator-forward kwargs
and accept injection (breaking change to `make_dace_backend`'s signature for
external callers passing translator knobs).
All pre-built backends keep identical names and behavior. Migrated the in-repo
tests that passed removed kwargs to `make_dace_translator(...)` injection and
added an injection unit test.
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.
Summary
factory-boy(a test-data library) was repurposed in production to compose the GTFN and DaCe backends and their OTF compilation workflows. Since every model is already a plain frozen dataclass, theTrait/SubFactory/SelfAttribute/LazyAttributemachinery and the stringly-typed__-path overrides added indirection without value.This PR (two commits) removes that and replaces it with plain builders + dependency injection:
It implements two items from the layered-architecture refactoring proposal: "plain-code backend builders, not factory-boy" and decoupling builders from their sub-components' internals.
Commit 1 — remove factory-boy
CompilerFactory,GTFNTranslationStepFactory,DaCeTranslationStepFactory,DaCeCompilationStepFactory.make_gtfn_workflow/make_gtfn_backend/make_dace_workflow/make_dace_backend. Traits becameboolkwargs;__-path overrides became real parameters.factory-boymoved from runtimedependenciesto thetestgroup (still used by the cartesian/eve IR test-data factories); stale mypy override removed;uv.lockrefreshed.Commit 2 — sub-component injection (lightweight DI)
Flattening the
__-paths into explicit maker kwargs re-coupled each maker to its children (most visiblymake_dace_backend, forwarding ~6 translator-only knobs). Commit 2 fixes that: a maker exposes only cross-cutting configuration (device, caching, auto-optimize, build type); a single-component knob is set by injecting that component.workflow.with_changes(step, **changes)— guarded helper that stamps cross-cutting fields (device_type; daceauto_optimize) onto a step only when it is a dataclass declaring them, so injecting an arbitrary callable stays safe.enable_itir_transforms/use_imperative_backend, gaintranslation/bindings/compilation/decorationinjection params.DaCeTranslatorgains defaults for its formerly-required fields; a newmake_dace_translator(...)owns the translator-local knobs and the relocatedoptimization_argsvalidation /unit_strides_kindderivation.make_dace_workflow/make_dace_backendshed their translator-forward kwargs and accept injection.Behavior & compatibility
run_gtfn_*,run_dace_*, and thegtfn_cpu/gtfn_gpure-exports) — verified field-by-field.make_dace_backendno longer accepts the translator-local kwargs (async_sdfg_call,optimization_args,use_metrics,use_zero_origin,unstructured_horizontal_has_unit_stride,use_max_domain_range_on_unstructured_shift). External callers (e.g. icon4py) switch tomake_dace_backend(..., translation=make_dace_translator(...)).Verification
uv run mypy src/clean; fullpre-commit(ruff, ruff-format, tach, mypy, license, uv-lock) green on all changed files.test_backend_inject_translation); the migrated DaCe CPU tests (test_dace_backend,test_dace_bindings) pass;make_dace_translator's validation paths checked directly.exec_alloc_descriptor1,test_dace_fastcall) can't run locally (nocupy/GPU) — needs a GPU runner for full-matrix confidence.Note on typing
Constructing
OTFCompileWorkflowdirectly surfaces twoCachedStep.persistent(...)calls where mypy cannot solve theHashTtype variable (it appears only in the fingerprinter's return) — previously hidden by factory-boy's dynamic typing. These carry a scoped, documented# type: ignore[arg-type].The
Backend → toolchainrename and the pipeline collapse from the larger refactor are intentionally left for follow-ups.