Deduplicate ExperimentConfig/read_config; move initial_condition to common#1353
Deduplicate ExperimentConfig/read_config; move initial_condition to common#1353jcanton wants to merge 2 commits into
Conversation
…ommon `ExperimentConfig` and the Fortran-namelist reader were duplicated between `standalone_driver` and `testing`, each marked "duplicate ... to avoid circular imports". The duplication existed because `testing` depended on `standalone_driver` (for `DriverConfig`/`InitialConditionConfig`), which is the wrong direction: the standalone driver should sit at the top of the dependency graph, with nothing depending on it. Changes: - New `icon4py.model.experiment_config` package holds the single `ExperimentConfig`, `DriverConfig`, and the shared `read_experiment_config` reader. It sits above the atmosphere packages but below both `testing` and `standalone_driver`, so neither depends on the other. The `testing -> standalone_driver` edge is removed. - `standalone_driver.config.read_config` and `testing.datatest_utils.create_experiment_configuration` become thin wrappers over the shared reader. - Move the `initial_condition` subpackage to `common/initial_condition/`, mirroring `common/topography/`, so the IC configs stay next to their compute functions. This required moving `StaticFieldFactories` (a NamedTuple of common factories) to `common/states/static_fields.py`. - Drop the now-unused `serialbox4py` dependency from `standalone_driver` (its only user moved to `common`, which already depends on it). - Remove the now-resolved `# type: ignore[arg-type]` / `TODO(1320)` workarounds in the standalone driver initial-condition tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@DropD I thought of getting this one in before it risks becoming annoying for your config PRs, hopefully it doesn't annoy you now, but if so we can postpone it to a later time |
|
Mandatory Tests Before merging, run the When developing, you can test your changes on CSCS CI before merge with the You can pass options to override pipeline variables, for example:
Available options are:
See The Optional Tests To run benchmarks you can use:
For more detailed information please look at CI in the EXCLAIM universe. |
|
cscs-ci run default;BACKENDS=gtfn_cpu;LEVELS=unit:integration |
There was a problem hiding this comment.
Pull request overview
This PR removes duplicated experiment-configuration code by introducing a shared icon4py.model.experiment_config package, and relocates initial-condition functionality into icon4py.model.common to fix an inverted dependency (testing -> standalone_driver) in the monorepo.
Changes:
- Add
model/experiment_configwith the canonicalExperimentConfig/DriverConfigand a sharedread_experiment_config(...)reader; updatetach.toml, workspace deps, and mypy paths accordingly. - Replace duplicated readers in
standalone_driver.config.read_configandtesting.datatest_utils.create_experiment_configurationwith thin wrappers over the shared reader. - Move initial-condition code (and
StaticFieldFactories) intoicon4py.model.common, and update standalone-driver code/tests to import from the new locations.
Reviewed changes
Copilot reviewed 21 out of 26 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds workspace member + dependency for icon4py-experiment-config; removes standalone-driver’s direct serialbox4py dep. |
| tach.toml | Adds icon4py.model.experiment_config module + adjusts dependency edges for testing and standalone_driver. |
| pyproject.toml | Registers new workspace member, dependency, and mypy/pytest paths; updates mypy override for moved IC package. |
| model/testing/src/icon4py/model/testing/definitions.py | Removes duplicated ExperimentConfig and imports the shared one. |
| model/testing/src/icon4py/model/testing/datatest_utils.py | Replaces local config assembly with read_experiment_config(...). |
| model/standalone_driver/tests/standalone_driver/mpi_tests/test_parallel_initial_conditions.py | Updates IC imports and removes now-unneeded type-ignore/TODO. |
| model/standalone_driver/tests/standalone_driver/integration_tests/test_initial_conditions.py | Updates IC imports and removes now-unneeded type-ignore/TODO. |
| model/standalone_driver/src/icon4py/model/standalone_driver/standalone_driver.py | Switches IC imports to icon4py.model.common.initial_condition. |
| model/standalone_driver/src/icon4py/model/standalone_driver/driver_states.py | Moves StaticFieldFactories to common and imports it from there. |
| model/standalone_driver/src/icon4py/model/standalone_driver/config.py | Re-exports shared config types and delegates read_config(...) to shared reader. |
| model/standalone_driver/pyproject.toml | Drops direct serialbox4py dependency. |
| model/experiment_config/src/icon4py/model/experiment_config/reader.py | New shared Fortran-namelist config reader assembling ExperimentConfig. |
| model/experiment_config/src/icon4py/model/experiment_config/config.py | New shared config dataclasses and type aliases for experiments/driver. |
| model/experiment_config/src/icon4py/model/experiment_config/init.py | New package metadata/version module. |
| model/experiment_config/src/icon4py/model/experiment_config/py.typed | Marks package as typed. |
| model/experiment_config/README.md | New package documentation. |
| model/experiment_config/pyproject.toml | New package build metadata and dependencies. |
| model/common/src/icon4py/model/common/topography/from_file.py | Updates documentation reference due to IC move. |
| model/common/src/icon4py/model/common/states/static_fields.py | New home for StaticFieldFactories. |
| model/common/src/icon4py/model/common/initial_condition/from_file.py | New file-based initial-condition implementation under common. |
| model/common/src/icon4py/model/common/initial_condition/config.py | Updates IC config + dispatcher to use new common IC subpackage layout. |
| model/common/src/icon4py/model/common/initial_condition/analytical/utils.py | Adds shared analytical-IC helper utilities under common. |
| model/common/src/icon4py/model/common/initial_condition/analytical/jablonowski_williamson.py | Updates imports/types to use common IC utilities + StaticFieldFactories. |
| model/common/src/icon4py/model/common/initial_condition/analytical/gauss3d.py | Updates imports/types to use common IC utilities + StaticFieldFactories. |
| model/common/src/icon4py/model/common/initial_condition/analytical/init.py | Adds package init for analytical IC submodule. |
| model/common/src/icon4py/model/common/initial_condition/init.py | Re-exports InitialConditionConfig and create from the new common location. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ignored so that this function shares the same call signature as the | ||
| analytical initial-condition functions (``jablonowski_williamson``, | ||
| ``gauss3d``, …) and can be stored transparently in | ||
| :attr:`~icon4py.model.standalone_driver.initial_condition.InitialConditionConfig.create`. | ||
| :attr:`~icon4py.model.common.initial_condition.create`. | ||
| """ |
| - `ExperimentConfig` / `DriverConfig` / `ProfilingStats` (`config.py`) | ||
| - the initial-condition configuration dataclasses (`initial_condition_config.py`) | ||
| - `read_experiment_config(...)`, which assembles an `ExperimentConfig` from a directory | ||
| of serialized Fortran namelists (`reader.py`) |
from a friend:
Closes #1320
What
ExperimentConfigand the Fortran-namelist config reader were duplicated betweenstandalone_driverandtesting, each carrying a# NOTE: duplicate ... to avoid circular importsmarker. The duplication existed becausetestingdepended onstandalone_driver(forDriverConfig/InitialConditionConfig) — the wrong direction, since the standalone driver should sit at the top of the dependency graph with nothing depending on it.Changes
icon4py.model.experiment_configpackage holding the singleExperimentConfig,DriverConfig, and the sharedread_experiment_configreader. It sits above the atmosphere packages but below bothtestingandstandalone_driver, so neither depends on the other — thetesting -> standalone_driveredge is gone.standalone_driver.config.read_configandtesting.datatest_utils.create_experiment_configurationare now thin wrappers over the shared reader.initial_conditionsubpackage tocommon/initial_condition/, mirroringcommon/topography/, so the IC config dataclasses stay next to their compute functions. This required relocatingStaticFieldFactories(aNamedTupleof common factories) tocommon/states/static_fields.py.serialbox4pydependency fromstandalone_driver(its only user moved tocommon, which already depends on it).# type: ignore[arg-type]/TODO(1320)workarounds in the standalone-driver initial-condition tests, since both sides now share a singleExperimentConfigtype.