diff --git a/docs/development/ADRs/next/0028-Plain-Builders-Instead-of-Factories.md b/docs/development/ADRs/next/0028-Plain-Builders-Instead-of-Factories.md new file mode 100644 index 0000000000..c141f847d3 --- /dev/null +++ b/docs/development/ADRs/next/0028-Plain-Builders-Instead-of-Factories.md @@ -0,0 +1,123 @@ +--- +tags: [backend, otf, toolchain, workflows, dependencies] +--- + +# Plain Builders Instead of factory-boy Factories + +- **Status**: valid +- **Authors**: Enrique González Paredes (@egparedes) +- **Created**: 2026-08-20 +- **Updated**: 2026-08-20 + +In the context of composing the GTFN and DaCe backends and their OTF compile +workflows, facing a production dependency on `factory-boy` — a test-data +library — whose `Trait` / `SubFactory` / `SelfAttribute` / `LazyAttribute` +machinery and stringly-typed `__`-path overrides are invisible to the type +checker, we decided to replace the factory classes with plain builder +functions over the existing frozen dataclasses, and to validate injected +sub-components explicitly, to achieve statically checked composition, one +fewer runtime dependency, and loud failures where the factories failed +silently. + +## Context + +Every object these factories build — `Backend`, `OTFCompileWorkflow`, +`GTFNTranslationStep`, `DaCeTranslator`, the compilers — is already a frozen +dataclass. `factory-boy` added a second, parallel construction language on top: + +- **Untyped.** The declarations are class attributes of a `Params` block, so + `mypy` cannot check them. `src/` carried **8** \`# type: ignore[assignment] + + # factory-boy typing not precise enough\` suppressions solely to keep the + + factories quiet. + +- **Silently wrong.** Overrides are `__`-delimited strings resolved at + runtime. When a path does not resolve, nothing happens. This was not + hypothetical: `run_gtfn_imperative` was declared as + + ```python + run_gtfn_imperative = GTFNBackendFactory( + name_postfix="_imperative", + otf_workflow__translation__use_imperative_backend=True, + ) + ``` + + but the `cached_translation` trait replaces `translation` with a + `CachedStep`, so the path never reached the wrapped `GTFNTranslationStep`. + The backend had `use_imperative_backend=False` — it was the declarative + backend under another name, and the `GTFN_CPU_IMPERATIVE` entry of the test + matrix had therefore never exercised imperative code generation. + `run_gtfn_no_transforms` was likewise named `run_gtfn_cpu`, colliding with + `run_gtfn`. + +- **A runtime dependency for a test-time concern.** `factory-boy` sat in + `[project] dependencies`, shipped to every user, to compose four backends. + +## Decision + +Factory classes are replaced by **plain builder functions**; `factory-boy` +moves to the `test` dependency group, where the `cartesian` and `eve` IR +test-data factories keep using it for what it is designed for. + +Builders follow two rules: + +1. **A builder takes cross-cutting configuration only** — device, caching, + build type, auto-optimize — and uses it to configure the steps it creates. +2. **An injected sub-component is used verbatim.** A caller that wants a + different translation step builds one and passes it; the builder never + reaches into it to stamp fields onto it. + +```python +run_gtfn_imperative = make_gtfn_backend( + name_postfix="_imperative", + translation=gtfn_module.GTFNTranslationStep(use_imperative_backend=True), +) +``` + +Rule 2 creates one hazard: an injected step could disagree with the +cross-cutting configuration — a CPU translation step in a GPU toolchain. +`workflow.check_device_agreement(step, device_type, what)` turns that into a +`ValueError` at construction time. It inspects only steps that structurally +declare a device (the `workflow.DeviceConfigurable` protocol) and is used to +**check**, never to mutate. + +We considered a `with_changes(step, **changes)` helper that stamps +cross-cutting fields onto whichever component is present, applying only the +fields the target declares. We rejected it: silently ignoring the fields a +target does not declare reproduces exactly the failure mode that motivated +this ADR — the `run_gtfn_imperative` bug is what a silent no-op looks like +after a year. Checking is the same amount of introspection with the opposite +failure mode. + +Builder defaults preserve the previous factory semantics: a standalone +compile-workflow builder leaves translation caching **off** (the +`cached_translation` trait was opt-in), while the backend builders turn it on. + +## Consequences + +- Composition is ordinary, statically checked Python. The 8 factory-related + `type: ignore` suppressions are gone, and a misspelled parameter is now a + `TypeError` at import rather than a silently ignored override. +- One fewer runtime dependency. +- **`run_gtfn_imperative` now actually uses the imperative backend.** This is + a behaviour change: the `GTFN_CPU_IMPERATIVE` test-matrix entry begins + exercising imperative code generation for the first time, and it + immediately fails on the pre-existing IR defect tracked in issue #2810. + Two call sites are xfailed against that issue (`test_hdiff` and + `test_concat_where::test_lap_like[static_domains]`); fixing the defect is + out of scope for a construction refactor. +- **`run_gtfn_no_transforms` is renamed** from `run_gtfn_cpu` to + `run_gtfn_cpu_no_transforms`, removing the collision with `run_gtfn`. No + cache is affected: 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` + reaches only the metrics source key and one error message, so what the + collision actually cost was two distinct backends sharing one metrics + identity. +- All other pre-built backends are unchanged, verified field-by-field against + the previous construction. +- Customizing a single knob of a sub-component now means building that + component, rather than passing a `__`-path string. This is more explicit and + slightly more verbose; `make_dace_backend` keeps its translator-local + keyword arguments so existing external callers are unaffected. diff --git a/docs/development/ADRs/next/README.md b/docs/development/ADRs/next/README.md index 1bf9d21812..f19167ef9b 100644 --- a/docs/development/ADRs/next/README.md +++ b/docs/development/ADRs/next/README.md @@ -52,6 +52,7 @@ Writing a new ADR is simple: - [0016 - Multiple Backends and Build Systems](0016-Multiple-Backends-and-Build-Systems.md) - [0017 - Toolchain Configuration](0017-Toolchain-Configuration.md) - [0027 - External Workspace Memory for DaCe Transients](0027-External_Workspace_Memory.md) +- [0028 - Plain Builders Instead of factory-boy Factories](0028-Plain-Builders-Instead-of-Factories.md) ### Python Integration diff --git a/docs/user/next/advanced/HackTheToolchain.md b/docs/user/next/advanced/HackTheToolchain.md index 15e2e98ff1..0d5e9ab74a 100644 --- a/docs/user/next/advanced/HackTheToolchain.md +++ b/docs/user/next/advanced/HackTheToolchain.md @@ -46,25 +46,28 @@ skip_linting_transforms = SkipLinting(**same_steps) skip_linting_transforms.step_order(DUMMY_FOP) ``` -## Alternative Factory +## Alternative Workflow + +Compile workflows are plain frozen dataclasses, so a variant is built by +replacing the steps you want to change on one the builders produced. ```python -class MyCodeGen: ... +import dataclasses -class Cpp2BindingsGen: ... +class MyCodeGen: ... -class PureCpp2WorkflowFactory(gtx.program_processors.runners.gtfn.GTFNCompileWorkflowFactory): - translation: workflow.Workflow[ - gtx.otf.stages.CompilableProgramDef, gtx.otf.artifacts.ProgramSource - ] = MyCodeGen() - bindings: workflow.Workflow[ - gtx.otf.artifacts.ProgramSource, gtx.otf.artifacts.ExtensionSource - ] = Cpp2BindingsGen() +class Cpp2BindingsGen: ... -PureCpp2WorkflowFactory(cmake_build_type=gtx.config.CMAKE_BUILD_TYPE.DEBUG) +pure_cpp2_workflow = dataclasses.replace( + gtx.program_processors.runners.gtfn.make_gtfn_compile_workflow( + cmake_build_type=gtx.config.CMakeBuildType.DEBUG + ), + translation=MyCodeGen(), + bindings=Cpp2BindingsGen(), +) ``` ## Invent new Workflow Types diff --git a/docs/user/next/advanced/WorkflowPatterns.md b/docs/user/next/advanced/WorkflowPatterns.md index 0e0abc4aea..ba6fd3a104 100644 --- a/docs/user/next/advanced/WorkflowPatterns.md +++ b/docs/user/next/advanced/WorkflowPatterns.md @@ -17,7 +17,6 @@ jupyter: import dataclasses import re -import factory import gt4py.next as gtx @@ -199,7 +198,7 @@ Let's say we want to make our calculation workflow compatible with string input. ```python editable=true slideshow={"slide_type": ""} # A plain conversion step turning a string into an int, chained into the -# workflow below and reused by `StrToIntFactory(cached=True)`. +# workflow below and reused by `make_str_to_int(cached=True)`. def to_int(inp: str) -> int: assert isinstance(inp, str), "Can not work with 'int'!" # yes, this is horribly contrived return int(inp) @@ -214,9 +213,9 @@ str_calc("1") -### Step with factory (builder) +### Step with a builder -If a step can be useful with different combinations of parameters and wrappers, it should have a factory. In this case we will add a neutral wrapper around it, so we can put any combination of wrappers into that: +If a step is useful with different combinations of parameters and wrappers, give it a **builder function**: a plain function taking the cross-cutting options and returning the assembled step. Steps are frozen dataclasses, so the builder is ordinary code — no factory framework involved, and the result is fully type-checked. @@ -229,32 +228,23 @@ class AnyStrToInt(gtx.otf.workflow.ChainableWorkflowMixin[str | int, int]): return self.inner_step(inp) -class StrToIntFactory(factory.Factory): - class Meta: - model = AnyStrToInt +def make_str_to_int( + *, cached: bool = False, step: gtx.otf.workflow.Workflow[str, int] = to_int +) -> AnyStrToInt: + if cached: + step = gtx.otf.workflow.CachedStep.in_memory(step=step, input_fingerprinter=str) + return AnyStrToInt(inner_step=step) - class Params: - default_step = to_int - cached = factory.Trait( - inner_step=factory.LazyAttribute( - lambda o: gtx.otf.workflow.CachedStep.in_memory( - step=o.default_step, input_fingerprinter=str - ) - ) - ) - inner_step = factory.LazyAttribute(lambda o: o.default_step) - - -cached = StrToIntFactory(cached=True) -uncached = StrToIntFactory() +cached = make_str_to_int(cached=True) +uncached = make_str_to_int() uncached.inner_step ``` ### Example in the Wild ```python -gtx.ffront.past_passes.linters.LinterFactory?? +gtx.ffront.past_passes.linters.linter_factory?? ``` @@ -413,5 +403,5 @@ gtx.program_processors.runners.gtfn.run_gtfn_gpu.executor.otf_workflow?? ``` ```python -gtx.program_processors.runners.gtfn.GTFNBackendFactory?? +gtx.program_processors.runners.gtfn.make_gtfn_backend?? ``` diff --git a/pyproject.toml b/pyproject.toml index e51bac0d90..8ca9a0021b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,6 +44,7 @@ profiling = [ scripts = ["pyyaml>=6.0.1", "typer>=0.16.0", "packaging"] test = [ 'coverage[toml]>=7.6.1', + 'factory-boy>=3.3.3', 'hypothesis>=6.0.0', 'nbmake>=1.4.6', 'nox>=2025.02.09', @@ -101,7 +102,6 @@ dependencies = [ 'dace>=2.0.0a7,<2.0.0a8', 'deepdiff>=8.1.0', 'devtools>=0.6', - 'factory-boy>=3.3.3', "filelock>=3.18.0", 'frozendict>=2.3', 'gridtools-cpp>=2.3.9,==2.*', @@ -260,12 +260,6 @@ module = 'gt4py.next.iterator.*' ignore_errors = true module = 'gt4py.next.iterator.runtime' -[[tool.mypy.overrides]] -ignore_missing_imports = true -implicit_reexport = true -# factory-boy is broken, see https://github.com/FactoryBoy/factory_boy/pull/1114 -module = "factory.*" - # -- pytest -- [tool.pytest] diff --git a/src/gt4py/next/otf/workflow.py b/src/gt4py/next/otf/workflow.py index 6e5bf42837..edd603f1b4 100644 --- a/src/gt4py/next/otf/workflow.py +++ b/src/gt4py/next/otf/workflow.py @@ -17,7 +17,7 @@ from typing_extensions import Self -from gt4py._core import filecache +from gt4py._core import definitions as core_defs, filecache from gt4py.eve.extended_typing import OpaqueMutableMapping from gt4py.next import config, fingerprinting, utils @@ -361,3 +361,37 @@ def __call__(self, inp: StartT) -> EndT: def cache_key(self, inp: StartT) -> str: return self.step_fingerprinter((self._step_fingerprint, self.input_fingerprinter(inp))) + + +@typing.runtime_checkable +class DeviceConfigurable(Protocol): + """A step that records the device it was configured for.""" + + device_type: core_defs.DeviceType + + +def check_device_agreement(step: Any, device_type: core_defs.DeviceType, what: str) -> None: + """ + Raise if an injected step is configured for a different device. + + Builders configure the steps they create themselves from the requested + device, but an injected step is used verbatim. Without this check a + mismatch would silently produce a pipeline whose steps disagree about the + target device, which surfaces much later as a confusing compilation or + runtime failure. + + Args: + step: The step to check. Steps that do not record a device are accepted. + device_type: The device the surrounding pipeline is built for. + what: Name of the step, used in the error message. + + Raises: + ValueError: If `step` records a device other than `device_type`. + """ + if isinstance(step, DeviceConfigurable) and step.device_type is not device_type: + raise ValueError( + f"The injected {what} is configured for device '{step.device_type.name}'," + f" but the workflow is being built for '{device_type.name}'. Build the step" + f" with 'device_type=DeviceType.{device_type.name}' or leave it out to get" + " the default." + ) diff --git a/src/gt4py/next/program_processors/codegens/gtfn/gtfn_module.py b/src/gt4py/next/program_processors/codegens/gtfn/gtfn_module.py index c32c1c15d1..b996be2626 100644 --- a/src/gt4py/next/program_processors/codegens/gtfn/gtfn_module.py +++ b/src/gt4py/next/program_processors/codegens/gtfn/gtfn_module.py @@ -12,7 +12,6 @@ import functools from typing import Any, Final, Optional -import factory import numpy as np from gt4py._core import definitions as core_defs @@ -286,13 +285,8 @@ def _not_implemented_for_device_type(self) -> NotImplementedError: ) -class GTFNTranslationStepFactory(factory.Factory[GTFNTranslationStep]): - class Meta: - model = GTFNTranslationStep +translate_program_cpu: Final[stages.TranslationStep] = GTFNTranslationStep() - -translate_program_cpu: Final[stages.TranslationStep] = GTFNTranslationStepFactory() # type: ignore[assignment] # factory-boy typing not precise enough - -translate_program_gpu: Final[stages.TranslationStep] = GTFNTranslationStepFactory( # type: ignore[assignment] # factory-boy typing not precise enough +translate_program_gpu: Final[stages.TranslationStep] = GTFNTranslationStep( device_type=core_defs.DeviceType.CUDA ) diff --git a/src/gt4py/next/program_processors/formatters/gtfn.py b/src/gt4py/next/program_processors/formatters/gtfn.py index 75494a1759..cea215e55b 100644 --- a/src/gt4py/next/program_processors/formatters/gtfn.py +++ b/src/gt4py/next/program_processors/formatters/gtfn.py @@ -16,7 +16,7 @@ @program_formatter.program_formatter def format_cpp(program: itir.Program, *args: Any, **kwargs: Any) -> str: - gtfn_translation = gtfn.GTFNCompileWorkflowFactory(cached_translation=False).translation + gtfn_translation = gtfn.make_gtfn_compile_workflow().translation assert isinstance(gtfn_translation, GTFNTranslationStep) return gtfn_translation.generate_stencil_source( program, diff --git a/src/gt4py/next/program_processors/runners/dace/workflow/backend.py b/src/gt4py/next/program_processors/runners/dace/workflow/backend.py index c0ea33daf0..9934ffdb4a 100644 --- a/src/gt4py/next/program_processors/runners/dace/workflow/backend.py +++ b/src/gt4py/next/program_processors/runners/dace/workflow/backend.py @@ -12,8 +12,6 @@ import warnings from typing import Any, Final -import factory - import gt4py.next.custom_layout_allocators as next_allocators from gt4py._core import definitions as core_defs from gt4py.next import backend, common, config @@ -40,43 +38,6 @@ def load_artifact(self, artifact: artifacts.CompilationArtifact) -> artifacts.Ex return program -class DaCeBackendFactory(factory.Factory): - """ - Workflow factory for the GTIR-DaCe backend. - - Several parameters are inherithed from `backend.Backend`, see below the specific ones. - - Args: - auto_optimize: Enables the SDFG transformation pipeline. - """ - - class Meta: - model = DaCeBackend - - class Params: - name_device = "cpu" - name_postfix = "" - gpu = factory.Trait( - allocator=next_allocators.StandardGPUFieldBufferAllocator(), - device_type=core_defs.CUPY_DEVICE_TYPE or core_defs.DeviceType.CUDA, - name_device="gpu", - ) - device_type = core_defs.DeviceType.CPU - otf_workflow = factory.SubFactory( - gtx_wfdfactory.DaCeWorkflowFactory, - cached_translation=True, - device_type=factory.SelfAttribute("..device_type"), - auto_optimize=factory.SelfAttribute("..auto_optimize"), - ) - auto_optimize = factory.Trait(name_postfix="_opt") - - name = factory.LazyAttribute(lambda o: f"run_dace_{o.name_device}{o.name_postfix}") - executor = factory.LazyAttribute(lambda o: o.otf_workflow) - allocator = next_allocators.StandardCPUFieldBufferAllocator() - transforms = backend.DEFAULT_TRANSFORMS - external_workspace = None - - def make_dace_backend( gpu: bool, auto_optimize: bool = True, @@ -154,16 +115,36 @@ def make_dace_backend( gtx_transformations.TransientMemoryMode.EXTERNAL ) - return DaCeBackendFactory( # type: ignore[return-value] # factory-boy typing not precise enough - gpu=gpu, + allocator: next_allocators.FieldBufferAllocatorProtocol + device_type: core_defs.DeviceType + if gpu: + allocator = next_allocators.StandardGPUFieldBufferAllocator() + device_type = core_defs.CUPY_DEVICE_TYPE or core_defs.DeviceType.CUDA + name_device = "gpu" + else: + allocator = next_allocators.StandardCPUFieldBufferAllocator() + device_type = core_defs.DeviceType.CPU + name_device = "cpu" + + translation = gtx_wfdfactory.make_dace_translator( + device_type=device_type, auto_optimize=auto_optimize, + auto_optimize_args=optimization_args, + async_sdfg_call=(async_sdfg_call if gpu else False), + unstructured_horizontal_has_unit_stride=unstructured_horizontal_has_unit_stride, + use_metrics=use_metrics, + disable_field_origin_on_program_arguments=use_zero_origin, + use_max_domain_range_on_unstructured_shift=use_max_domain_range_on_unstructured_shift, + ) + + return DaCeBackend( + name=f"run_dace_{name_device}{'_opt' if auto_optimize else ''}", + executor=gtx_wfdfactory.make_dace_compile_workflow( + device_type=device_type, cached_translation=True, translation=translation + ), + allocator=allocator, + transforms=backend.DEFAULT_TRANSFORMS, external_workspace=external_workspace, - otf_workflow__bare_translation__async_sdfg_call=(async_sdfg_call if gpu else False), - otf_workflow__bare_translation__auto_optimize_args=optimization_args, - otf_workflow__bare_translation__unstructured_horizontal_has_unit_stride=unstructured_horizontal_has_unit_stride, - otf_workflow__bare_translation__use_metrics=use_metrics, - otf_workflow__bare_translation__disable_field_origin_on_program_arguments=use_zero_origin, - otf_workflow__bare_translation__use_max_domain_range_on_unstructured_shift=use_max_domain_range_on_unstructured_shift, ) diff --git a/src/gt4py/next/program_processors/runners/dace/workflow/compilation.py b/src/gt4py/next/program_processors/runners/dace/workflow/compilation.py index 6b2683b7ef..78fff3b84b 100644 --- a/src/gt4py/next/program_processors/runners/dace/workflow/compilation.py +++ b/src/gt4py/next/program_processors/runners/dace/workflow/compilation.py @@ -18,7 +18,6 @@ import dace import dace.codegen.compiler as dace_compiler -import factory from gt4py._core import definitions as core_defs, locking from gt4py.eve import extended_typing as xtyping @@ -353,8 +352,3 @@ def __call__(self, inp: SDFGExtensionSource) -> DaCeCompilationArtifact: bind_func_name=self.bind_func_name, device_type=self.device_type, ) - - -class DaCeCompilationStepFactory(factory.Factory): - class Meta: - model = DaCeCompiler diff --git a/src/gt4py/next/program_processors/runners/dace/workflow/factory.py b/src/gt4py/next/program_processors/runners/dace/workflow/factory.py index 2f37f90cd7..776d40df61 100644 --- a/src/gt4py/next/program_processors/runners/dace/workflow/factory.py +++ b/src/gt4py/next/program_processors/runners/dace/workflow/factory.py @@ -9,68 +9,128 @@ from __future__ import annotations import functools -from typing import Final - -import factory +from typing import Any, Final from gt4py._core import definitions as core_defs, filecache from gt4py.next import config, fingerprinting -from gt4py.next.otf import recipes, workflow +from gt4py.next.otf import artifacts, recipes, stages, workflow from gt4py.next.otf.compilation import cache from gt4py.next.program_processors.runners.dace.workflow import bindings as bindings_step -from gt4py.next.program_processors.runners.dace.workflow.compilation import ( - DaCeCompilationStepFactory, -) -from gt4py.next.program_processors.runners.dace.workflow.translation import ( - DaCeTranslationStepFactory, -) +from gt4py.next.program_processors.runners.dace.workflow.compilation import DaCeCompiler +from gt4py.next.program_processors.runners.dace.workflow.translation import DaCeTranslator _GT_DACE_BINDING_FUNCTION_NAME: Final[str] = "update_sdfg_args" -class DaCeWorkflowFactory(factory.Factory): - class Meta: - model = recipes.OTFCompileWorkflow +def make_dace_translator( + *, + device_type: core_defs.DeviceType = core_defs.DeviceType.CPU, + auto_optimize: bool = False, + auto_optimize_args: dict[str, Any] | None = None, + async_sdfg_call: bool = False, + unstructured_horizontal_has_unit_stride: bool = False, + use_metrics: bool = True, + disable_field_origin_on_program_arguments: bool = False, + use_max_domain_range_on_unstructured_shift: bool | None = None, +) -> DaCeTranslator: + """ + Build the GTIR -> SDFG translation step. + + Args: + device_type: The device the compiled program targets. + auto_optimize: Enable the SDFG auto-optimize pipeline. + auto_optimize_args: Configuration for the auto-optimize pipeline. + async_sdfg_call: Make an asynchronous SDFG call on GPU. + unstructured_horizontal_has_unit_stride: Replace the field stride symbol + with '1' in the horizontal dimension. + use_metrics: Add SDFG instrumentation for stencil compute time. + disable_field_origin_on_program_arguments: Assume zero-based field origins. + use_max_domain_range_on_unstructured_shift: See `DaCeTranslator`. + + Returns: + The configured translation step. + """ + return DaCeTranslator( + device_type=device_type, + auto_optimize=auto_optimize, + auto_optimize_args=auto_optimize_args, + async_sdfg_call=async_sdfg_call, + unstructured_horizontal_has_unit_stride=unstructured_horizontal_has_unit_stride, + use_metrics=use_metrics, + disable_field_origin_on_program_arguments=disable_field_origin_on_program_arguments, + use_max_domain_range_on_unstructured_shift=use_max_domain_range_on_unstructured_shift, + ) + + +def make_dace_compile_workflow( + *, + device_type: core_defs.DeviceType = core_defs.DeviceType.CPU, + auto_optimize: bool = False, + cached_translation: bool = False, + cmake_build_type: config.CMakeBuildType | None = None, + translation: DaCeTranslator | None = None, +) -> recipes.OTFCompileWorkflow: + """ + Build the DaCe translation -> bindings -> compilation workflow. + + Cross-cutting configuration is passed as keyword arguments and used to + configure the steps this function creates. To customize the translation + step, build one with `make_dace_translator` and pass it as `translation`; + it is used verbatim, so it must agree with `device_type`. - class Params: - auto_optimize: bool = False - device_type: core_defs.DeviceType = core_defs.DeviceType.CPU - cmake_build_type: config.CMakeBuildType = factory.LazyFunction( # type: ignore[assignment] # factory-boy typing not precise enough - lambda: config.CMAKE_BUILD_TYPE + Args: + device_type: The device the compiled program targets. + auto_optimize: Enable the SDFG auto-optimize pipeline in the default + translation step. + cached_translation: Wrap the translation step in a persistent cache. + Off by default; `make_dace_backend` turns it on. + cmake_build_type: Build type for the generated project. Defaults to the + value in `config`. + translation: A pre-built translation step. + + Returns: + The composed compile workflow. + + Raises: + ValueError: If `translation` is configured for a different device. + """ + if cmake_build_type is None: + cmake_build_type = config.CMAKE_BUILD_TYPE + + if translation is None: + bare_translation = make_dace_translator( + device_type=device_type, auto_optimize=auto_optimize ) + else: + workflow.check_device_agreement(translation, device_type, "DaCe translation step") + bare_translation = translation - cached_translation = factory.Trait( - translation=factory.LazyAttribute( - lambda o: workflow.CachedStep.persistent( - o.bare_translation, - input_fingerprinter=fingerprinting.strict_fingerprinter, - cache=filecache.FileCache( - cache.get_translation_cache_folder( - cache.get_cache_base_path(config.BUILD_CACHE_LIFETIME), "dace" - ) - ), + translation_step: stages.TranslationStep + if cached_translation: + translation_step = workflow.CachedStep[ + stages.CompilableProgramDef, artifacts.ProgramSource, str + ].persistent( + bare_translation, + input_fingerprinter=fingerprinting.strict_fingerprinter, + cache=filecache.FileCache( + cache.get_translation_cache_folder( + cache.get_cache_base_path(config.BUILD_CACHE_LIFETIME), "dace" ) ), ) + else: + translation_step = bare_translation - bare_translation = factory.SubFactory( - DaCeTranslationStepFactory, - device_type=factory.SelfAttribute("..device_type"), - auto_optimize=factory.SelfAttribute("..auto_optimize"), - ) - - translation = factory.LazyAttribute(lambda o: o.bare_translation) - bindings = factory.LazyAttribute( - lambda o: functools.partial( - bindings_step.bind_sdfg, + return recipes.OTFCompileWorkflow( + translation=translation_step, + bindings=functools.partial( + bindings_step.bind_sdfg, bind_func_name=_GT_DACE_BINDING_FUNCTION_NAME + ), + compilation=DaCeCompiler( bind_func_name=_GT_DACE_BINDING_FUNCTION_NAME, - ) - ) - compilation = factory.SubFactory( - DaCeCompilationStepFactory, - bind_func_name=_GT_DACE_BINDING_FUNCTION_NAME, - cache_lifetime=factory.LazyFunction(lambda: config.BUILD_CACHE_LIFETIME), - device_type=factory.SelfAttribute("..device_type"), - cmake_build_type=factory.SelfAttribute("..cmake_build_type"), + cache_lifetime=config.BUILD_CACHE_LIFETIME, + device_type=device_type, + cmake_build_type=cmake_build_type, + ), ) diff --git a/src/gt4py/next/program_processors/runners/dace/workflow/translation.py b/src/gt4py/next/program_processors/runners/dace/workflow/translation.py index f26e982eff..e45f3840b3 100644 --- a/src/gt4py/next/program_processors/runners/dace/workflow/translation.py +++ b/src/gt4py/next/program_processors/runners/dace/workflow/translation.py @@ -12,7 +12,6 @@ from typing import Any, Optional import dace -import factory from gt4py._core import definitions as core_defs from gt4py.next import common @@ -467,8 +466,3 @@ def __call__( code_spec=artifacts.SDFGCodeSpec(), ) return module - - -class DaCeTranslationStepFactory(factory.Factory): - class Meta: - model = DaCeTranslator diff --git a/src/gt4py/next/program_processors/runners/gtfn.py b/src/gt4py/next/program_processors/runners/gtfn.py index c48caab305..8d9a4fe435 100644 --- a/src/gt4py/next/program_processors/runners/gtfn.py +++ b/src/gt4py/next/program_processors/runners/gtfn.py @@ -10,7 +10,6 @@ import pathlib from typing import Any -import factory import numpy as np import gt4py._core.definitions as core_defs @@ -19,7 +18,7 @@ from gt4py.next import backend, common, config, field_utils, fingerprinting from gt4py.next.embedded import nd_array_field from gt4py.next.instrumentation import metrics -from gt4py.next.otf import artifacts, recipes, workflow +from gt4py.next.otf import artifacts, recipes, stages, workflow from gt4py.next.otf.binding import nanobind from gt4py.next.otf.compilation import cache, compiler from gt4py.next.otf.compilation.build_systems import compiledb @@ -123,96 +122,143 @@ def _make_artifact( ) -class GTFNCompilerFactory(factory.Factory): - class Meta: - model = GTFNCompiler - - -class GTFNCompileWorkflowFactory(factory.Factory): - class Meta: - model = recipes.OTFCompileWorkflow - - class Params: - device_type: core_defs.DeviceType = core_defs.DeviceType.CPU - cmake_build_type: config.CMakeBuildType = factory.LazyFunction( # type: ignore[assignment] # factory-boy typing not precise enough - lambda: config.CMAKE_BUILD_TYPE - ) - unstructured_horizontal_has_unit_stride: bool = factory.LazyFunction( # type: ignore[assignment] # factory-boy typing not precise enough - lambda: config.UNSTRUCTURED_HORIZONTAL_HAS_UNIT_STRIDE - ) - builder_factory: compiler.BuildSystemProjectGenerator = factory.LazyAttribute( # type: ignore[assignment] # factory-boy typing not precise enough - lambda o: compiledb.CompiledbFactory(cmake_build_type=o.cmake_build_type) - ) - - cached_translation = factory.Trait( - translation=factory.LazyAttribute( - lambda o: workflow.CachedStep.persistent( - o.bare_translation, - input_fingerprinter=fingerprinting.strict_fingerprinter, - cache=filecache.FileCache( - cache.get_translation_cache_folder( - cache.get_cache_base_path(config.BUILD_CACHE_LIFETIME), "gtfn" - ) - ), +def make_gtfn_compile_workflow( + *, + device_type: core_defs.DeviceType = core_defs.DeviceType.CPU, + cached_translation: bool = False, + cmake_build_type: config.CMakeBuildType | None = None, + unstructured_horizontal_has_unit_stride: bool | None = None, + builder_factory: compiler.BuildSystemProjectGenerator | None = None, + translation: gtfn_module.GTFNTranslationStep | None = None, +) -> recipes.OTFCompileWorkflow: + """ + Build the GTFN translation -> bindings -> compilation workflow. + + Cross-cutting configuration (device, translation caching, build type) is + passed as keyword arguments and used to configure the steps this function + creates. To customize the translation step, build one and pass it as + `translation`; it is used verbatim, so it must agree with `device_type`. + + Args: + device_type: The device the compiled program targets. + cached_translation: Wrap the translation step in a persistent cache. + Off by default; `make_gtfn_backend` turns it on. + cmake_build_type: Build type for the generated CMake project. Defaults + to the value in `config`. + unstructured_horizontal_has_unit_stride: Layout assumption passed to the + bindings generator. Defaults to the value in `config`. + builder_factory: Build-system project generator. Defaults to a + `CompiledbFactory` using `cmake_build_type`. + translation: A pre-built translation step. Defaults to a + `GTFNTranslationStep` configured for `device_type`. + + Returns: + The composed compile workflow. + + Raises: + ValueError: If `translation` is configured for a different device. + """ + if cmake_build_type is None: + cmake_build_type = config.CMAKE_BUILD_TYPE + if unstructured_horizontal_has_unit_stride is None: + unstructured_horizontal_has_unit_stride = config.UNSTRUCTURED_HORIZONTAL_HAS_UNIT_STRIDE + if builder_factory is None: + builder_factory = compiledb.CompiledbFactory(cmake_build_type=cmake_build_type) + + if translation is None: + bare_translation = gtfn_module.GTFNTranslationStep(device_type=device_type) + else: + workflow.check_device_agreement(translation, device_type, "GTFN translation step") + bare_translation = translation + + translation_step: stages.TranslationStep + if cached_translation: + translation_step = workflow.CachedStep[ + stages.CompilableProgramDef, artifacts.ProgramSource, str + ].persistent( + bare_translation, + input_fingerprinter=fingerprinting.strict_fingerprinter, + cache=filecache.FileCache( + cache.get_translation_cache_folder( + cache.get_cache_base_path(config.BUILD_CACHE_LIFETIME), "gtfn" ) ), ) - - bare_translation = factory.SubFactory( - gtfn_module.GTFNTranslationStepFactory, - device_type=factory.SelfAttribute("..device_type"), - ) - - translation = factory.LazyAttribute(lambda o: o.bare_translation) - bindings: workflow.Workflow[artifacts.ProgramSource, artifacts.ExtensionSource] = ( - factory.LazyAttribute( # type: ignore[assignment] # factory-boy typing not precise enough - lambda o: nanobind.ExtensionGenerator( - unstructured_horizontal_has_unit_stride=o.unstructured_horizontal_has_unit_stride - ) - ) - ) - compilation = factory.SubFactory( - GTFNCompilerFactory, - cache_lifetime=factory.LazyFunction(lambda: config.BUILD_CACHE_LIFETIME), - builder_factory=factory.SelfAttribute("..builder_factory"), - device_type=factory.SelfAttribute("..device_type"), + else: + translation_step = bare_translation + + return recipes.OTFCompileWorkflow( + translation=translation_step, + # `OTFCompileWorkflow` is not parameterized over the code spec, so its + # `bindings` field is typed for `ProgramSource[Any]` while + # `ExtensionGenerator` accepts only C++-like specs. Parameterizing the + # pipeline is the real fix and belongs with the pipeline rework. + bindings=nanobind.ExtensionGenerator( # type: ignore[arg-type] # see comment above + unstructured_horizontal_has_unit_stride=unstructured_horizontal_has_unit_stride + ), + compilation=GTFNCompiler( + cache_lifetime=config.BUILD_CACHE_LIFETIME, + builder_factory=builder_factory, + device_type=device_type, + ), ) -class GTFNBackendFactory(factory.Factory): - class Meta: - model = backend.Backend - - class Params: - name_device = "cpu" - name_postfix = "" - gpu = factory.Trait( - allocator=next_allocators.StandardGPUFieldBufferAllocator(), - device_type=core_defs.CUPY_DEVICE_TYPE or core_defs.DeviceType.CUDA, - name_device="gpu", - ) +def make_gtfn_backend( + *, + gpu: bool = False, + name_postfix: str = "", + translation: gtfn_module.GTFNTranslationStep | None = None, + executor: workflow.Workflow[stages.CompilableProgramDef, artifacts.CompilationArtifact] + | None = None, +) -> backend.Backend: + """ + Build a GTFN backend for the given device. + + Args: + gpu: Target the GPU instead of the CPU. + name_postfix: Appended to the backend name, which must stay unique. + translation: A pre-built translation step, forwarded to + `make_gtfn_compile_workflow`. + executor: A pre-built compile workflow, replacing the default one. + + Returns: + The configured backend. + """ + allocator: next_allocators.FieldBufferAllocatorProtocol + device_type: core_defs.DeviceType + if gpu: + allocator = next_allocators.StandardGPUFieldBufferAllocator() + device_type = core_defs.CUPY_DEVICE_TYPE or core_defs.DeviceType.CUDA + name_device = "gpu" + else: + allocator = next_allocators.StandardCPUFieldBufferAllocator() device_type = core_defs.DeviceType.CPU - otf_workflow = factory.SubFactory( - GTFNCompileWorkflowFactory, - cached_translation=True, - device_type=factory.SelfAttribute("..device_type"), + name_device = "cpu" + + if executor is None: + executor = make_gtfn_compile_workflow( + device_type=device_type, cached_translation=True, translation=translation ) - name = factory.LazyAttribute(lambda o: f"run_gtfn_{o.name_device}{o.name_postfix}") - executor = factory.LazyAttribute(lambda o: o.otf_workflow) - allocator = next_allocators.StandardCPUFieldBufferAllocator() - transforms = backend.DEFAULT_TRANSFORMS + return backend.Backend( + name=f"run_gtfn_{name_device}{name_postfix}", + executor=executor, + allocator=allocator, + transforms=backend.DEFAULT_TRANSFORMS, + ) -run_gtfn = GTFNBackendFactory() +run_gtfn = make_gtfn_backend() -run_gtfn_imperative = GTFNBackendFactory( +run_gtfn_imperative = make_gtfn_backend( name_postfix="_imperative", - otf_workflow__translation__use_imperative_backend=True, + translation=gtfn_module.GTFNTranslationStep(use_imperative_backend=True), ) -run_gtfn_gpu = GTFNBackendFactory(gpu=True) +run_gtfn_gpu = make_gtfn_backend(gpu=True) -run_gtfn_no_transforms = GTFNBackendFactory( - otf_workflow__bare_translation__enable_itir_transforms=False +run_gtfn_no_transforms = make_gtfn_backend( + name_postfix="_no_transforms", + translation=gtfn_module.GTFNTranslationStep(enable_itir_transforms=False), ) diff --git a/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_concat_where.py b/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_concat_where.py index 577f5a520e..d849b017c6 100644 --- a/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_concat_where.py +++ b/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_concat_where.py @@ -335,7 +335,19 @@ def testee(interior: cases.KField, boundary: cases.KField) -> cases.KField: ) -def test_lap_like(cartesian_case, static_domains: bool): +def test_lap_like(request, cartesian_case, static_domains: bool): + if static_domains and "imperative" in getattr(cartesian_case.backend, "name", ""): + # The imperative code path leaves the CSE temporaries undeclared, so symbol + # validation rejects the IR. See https://github.com/GridTools/gt4py/issues/2810. + # Only the static-domain variant folds enough to trigger it; `dynamic_domains` + # passes and is deliberately left running. + # Strict, so that fixing the issue fails here instead of silently passing. + request.applymarker( + pytest.mark.xfail( + strict=True, reason="GTFN imperative backend does not declare CSE temporaries." + ) + ) + @gtx.field_operator(static_domains=static_domains) def testee( inp: cases.IJField, boundary: np.int32, shape: tuple[np.int32, np.int32] diff --git a/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_temporaries_with_sizes.py b/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_temporaries_with_sizes.py index 80963d83ae..f13d2acb8c 100644 --- a/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_temporaries_with_sizes.py +++ b/tests/next_tests/integration_tests/feature_tests/ffront_tests/test_temporaries_with_sizes.py @@ -34,8 +34,8 @@ def exec_alloc_descriptor(): return backend.Backend( name="run_gtfn_with_temporaries_and_sizes", transforms=backend.DEFAULT_TRANSFORMS, - executor=gtfn.GTFNCompileWorkflowFactory( - translation=gtfn.gtfn_module.GTFNTranslationStepFactory( + executor=gtfn.make_gtfn_compile_workflow( + translation=gtfn.gtfn_module.GTFNTranslationStep( symbolic_domain_sizes={ "Cell": "num_cells", "Edge": "num_edges", diff --git a/tests/next_tests/integration_tests/multi_feature_tests/iterator_tests/test_hdiff.py b/tests/next_tests/integration_tests/multi_feature_tests/iterator_tests/test_hdiff.py index 3ebacfd80e..37538aab04 100644 --- a/tests/next_tests/integration_tests/multi_feature_tests/iterator_tests/test_hdiff.py +++ b/tests/next_tests/integration_tests/multi_feature_tests/iterator_tests/test_hdiff.py @@ -63,9 +63,19 @@ def hdiff(inp, coeff, out, x, y): @pytest.mark.uses_lift @pytest.mark.uses_origin -def test_hdiff(hdiff_reference, program_processor): +def test_hdiff(request, hdiff_reference, program_processor): program_processor, validate = program_processor + if "imperative" in getattr(program_processor, "name", ""): + # The imperative code path leaves the CSE temporaries undeclared, so symbol + # validation rejects the IR. See https://github.com/GridTools/gt4py/issues/2810. + # Strict, so that fixing the issue fails here instead of silently passing. + request.applymarker( + pytest.mark.xfail( + strict=True, reason="GTFN imperative backend does not declare CSE temporaries." + ) + ) + inp, coeff, out = hdiff_reference shape = (out.shape[0], out.shape[1]) diff --git a/tests/next_tests/unit_tests/otf_tests/test_compiled_program.py b/tests/next_tests/unit_tests/otf_tests/test_compiled_program.py index 9b3f4bc2cb..abf6ad73dd 100644 --- a/tests/next_tests/unit_tests/otf_tests/test_compiled_program.py +++ b/tests/next_tests/unit_tests/otf_tests/test_compiled_program.py @@ -130,7 +130,7 @@ def pirate(program: workflow.ConcreteArtifact): hijacked_program = program return _NoOpArtifact() - hacked_gtfn_backend = gtfn.GTFNBackendFactory(name_postfix="_custom", executor=pirate) + hacked_gtfn_backend = gtfn.make_gtfn_backend(name_postfix="_custom", executor=pirate) testee = testee_prog.with_backend(hacked_gtfn_backend).compile(cond=[True], offset_provider={}) testee( diff --git a/tests/next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py b/tests/next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py index fc077b3a90..2f05f8debd 100644 --- a/tests/next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py +++ b/tests/next_tests/unit_tests/program_processor_tests/codegens_tests/gtfn_tests/test_gtfn_module.py @@ -134,11 +134,11 @@ def test_gtfn_file_cache(program_example): data=fencil, args=arguments.CompileTimeArgs.from_concrete(*parameters, **{"offset_provider": {}}), ) - cached_gtfn_translation_step = gtfn.GTFNCompileWorkflowFactory( + cached_gtfn_translation_step = gtfn.make_gtfn_compile_workflow( cached_translation=True ).translation - bare_gtfn_translation_step = gtfn.GTFNCompileWorkflowFactory( + bare_gtfn_translation_step = gtfn.make_gtfn_compile_workflow( cached_translation=False ).translation diff --git a/tests/next_tests/unit_tests/program_processor_tests/runners_tests/test_gtfn.py b/tests/next_tests/unit_tests/program_processor_tests/runners_tests/test_gtfn.py index cb72c80f42..5252f45565 100644 --- a/tests/next_tests/unit_tests/program_processor_tests/runners_tests/test_gtfn.py +++ b/tests/next_tests/unit_tests/program_processor_tests/runners_tests/test_gtfn.py @@ -29,9 +29,9 @@ from gt4py.next.program_processors.runners import gtfn -def test_backend_factory_trait_device(): - cpu_version = gtfn.GTFNBackendFactory(gpu=False) - gpu_version = gtfn.GTFNBackendFactory(gpu=True) +def test_make_gtfn_backend_trait_device(): + cpu_version = gtfn.make_gtfn_backend(gpu=False) + gpu_version = gtfn.make_gtfn_backend(gpu=True) assert cpu_version.name == "run_gtfn_cpu" assert isinstance(cpu_version.executor.translation, workflow.CachedStep) @@ -52,11 +52,11 @@ def test_backend_factory_trait_device(): ) -def test_backend_factory_build_cache_config(monkeypatch): +def test_make_gtfn_backend_build_cache_config(monkeypatch): monkeypatch.setattr(config, "BUILD_CACHE_LIFETIME", config.BuildCacheLifetime.SESSION) - session_version = gtfn.GTFNBackendFactory() + session_version = gtfn.make_gtfn_backend() monkeypatch.setattr(config, "BUILD_CACHE_LIFETIME", config.BuildCacheLifetime.PERSISTENT) - persistent_version = gtfn.GTFNBackendFactory() + persistent_version = gtfn.make_gtfn_backend() assert session_version.executor.compilation.cache_lifetime is config.BuildCacheLifetime.SESSION assert ( @@ -65,11 +65,11 @@ def test_backend_factory_build_cache_config(monkeypatch): ) -def test_backend_factory_build_type_config(monkeypatch): +def test_make_gtfn_backend_build_type_config(monkeypatch): monkeypatch.setattr(config, "CMAKE_BUILD_TYPE", config.CMakeBuildType.RELEASE) - release_version = gtfn.GTFNBackendFactory() + release_version = gtfn.make_gtfn_backend() monkeypatch.setattr(config, "CMAKE_BUILD_TYPE", config.CMakeBuildType.MIN_SIZE_REL) - min_size_version = gtfn.GTFNBackendFactory() + min_size_version = gtfn.make_gtfn_backend() assert ( release_version.executor.compilation.builder_factory.cmake_build_type @@ -90,9 +90,9 @@ def test_cmake_build_type_changes_build_folder(monkeypatch, tmp_path): land in different cache folders. """ monkeypatch.setattr(config, "CMAKE_BUILD_TYPE", config.CMakeBuildType.RELEASE) - release_version = gtfn.GTFNBackendFactory() + release_version = gtfn.make_gtfn_backend() monkeypatch.setattr(config, "CMAKE_BUILD_TYPE", config.CMakeBuildType.DEBUG) - debug_version = gtfn.GTFNBackendFactory() + debug_version = gtfn.make_gtfn_backend() release_compiler = release_version.executor.compilation debug_compiler = debug_version.executor.compilation diff --git a/uv.lock b/uv.lock index 3b81092dab..daffe9ca10 100644 --- a/uv.lock +++ b/uv.lock @@ -1346,7 +1346,6 @@ dependencies = [ { name = "dace" }, { name = "deepdiff" }, { name = "devtools" }, - { name = "factory-boy" }, { name = "filelock" }, { name = "frozendict" }, { name = "gridtools-cpp" }, @@ -1426,6 +1425,7 @@ dev = [ { name = "coverage" }, { name = "cython" }, { name = "esbonio" }, + { name = "factory-boy" }, { name = "hypothesis" }, { name = "jupytext" }, { name = "matplotlib" }, @@ -1492,6 +1492,7 @@ scripts = [ ] test = [ { name = "coverage" }, + { name = "factory-boy" }, { name = "hypothesis" }, { name = "nbmake" }, { name = "nox" }, @@ -1541,7 +1542,6 @@ requires-dist = [ { name = "dace", specifier = ">=2.0.0a7,<2.0.0a8" }, { name = "deepdiff", specifier = ">=8.1.0" }, { name = "devtools", specifier = ">=0.6" }, - { name = "factory-boy", specifier = ">=3.3.3" }, { name = "filelock", specifier = ">=3.18.0" }, { name = "frozendict", specifier = ">=2.3" }, { name = "gridtools-cpp", specifier = "==2.*,>=2.3.9" }, @@ -1587,6 +1587,7 @@ dev = [ { name = "coverage", extras = ["toml"], specifier = ">=7.6.1" }, { name = "cython", specifier = ">=3.0.0" }, { name = "esbonio", specifier = ">=0.16.0" }, + { name = "factory-boy", specifier = ">=3.3.3" }, { name = "hypothesis", specifier = ">=6.0.0" }, { name = "jupytext", specifier = ">=1.14" }, { name = "matplotlib", specifier = ">=3.9.0" }, @@ -1651,6 +1652,7 @@ scripts = [ ] test = [ { name = "coverage", extras = ["toml"], specifier = ">=7.6.1" }, + { name = "factory-boy", specifier = ">=3.3.3" }, { name = "hypothesis", specifier = ">=6.0.0" }, { name = "nbmake", specifier = ">=1.4.6" }, { name = "nox", specifier = ">=2025.2.9" },