refactor standalone_driver#1304
Conversation
* main: Update development status from alpha to beta (#1285) Py2fgen: fix fortran types, generate header only on demand (#1275) enforce max 5 positional arguments with ruff (#1270) Tracer advection exchange (#1050) Replace one-argument wheres with nonzero (#1286) DaCe backend: set unstructured_horizontal_has_unit_stride=True (#1130) Remove hardcoded numpy use from `grid_refinement.py` and `metric_fields.py` (#1282) Bump version to v0.2.0rc2 (#1284)
There was a problem hiding this comment.
Pull request overview
Refactors the standalone driver’s entry points by separating a dedicated Typer-based CLI from a test-friendly driver runner API, while enabling targeted overrides of configuration values (e.g., end date / number of timesteps) to restore prior test tolerances and behavior.
Changes:
- Introduces
standalone_driver.run_driver()/ refactorsinitialize_driver()to accept already-parsed config + grid manager + process props, avoiding CLI-oriented side effects in tests. - Updates driver time handling to support multiple “end simulation” modes (
num_timesteps,relative_time,absolute_time) and improves logging of run configuration. - Updates standalone driver integration and MPI tests to use config overrides and the new API, and updates the package script entry point to the Typer
app.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| model/testing/src/icon4py/model/testing/fixtures/datatest.py | Adjusts datatest fixtures; experiment parametrization now (intended to) ensure serialized data is present before config usage. |
| model/testing/src/icon4py/model/testing/definitions.py | Adds ExperimentConfig.with_overrides() helper for test-side config patching. |
| model/standalone_driver/tests/standalone_driver/mpi_tests/test_parallel_standalone_driver.py | Migrates MPI standalone-driver comparison test to read_config() + with_overrides() + run_driver(). |
| model/standalone_driver/tests/standalone_driver/mpi_tests/test_parallel_initial_conditions.py | Migrates MPI initial-condition comparison test to new config/grid-manager initialization flow. |
| model/standalone_driver/tests/standalone_driver/integration_tests/test_standalone_driver.py | Migrates integration test to run_driver() and uses end_simulation override to match expected savepoints/tolerances. |
| model/standalone_driver/tests/standalone_driver/integration_tests/test_initial_conditions.py | Migrates integration initial-conditions test to config overrides + grid-manager based initialization. |
| model/standalone_driver/src/icon4py/model/standalone_driver/standalone_driver.py | Refactors driver initialization API and adds run_driver() wrapper that creates ICs + runs integration. |
| model/standalone_driver/src/icon4py/model/standalone_driver/main.py | Converts CLI to a Typer app command and reimplements CLI execution via read_config() + overrides + run_driver(). |
| model/standalone_driver/src/icon4py/model/standalone_driver/driver_utils.py | Updates driver setup logging to reflect the new time variable model and end-simulation mode. |
| model/standalone_driver/src/icon4py/model/standalone_driver/driver_states.py | Reworks time bookkeeping (ModelTimeVariables) around end_simulation rather than fixed start/end dates. |
| model/standalone_driver/src/icon4py/model/standalone_driver/config.py | Introduces EndSimulation type aliases, replaces end_date with end_simulation, and adds ExperimentConfig.with_overrides(). |
| model/standalone_driver/pyproject.toml | Updates console-script entry point to main:app (Typer application). |
Comments suppressed due to low confidence (1)
model/standalone_driver/tests/standalone_driver/mpi_tests/test_parallel_initial_conditions.py:28
datatest_utils as dt_utilsis imported but no longer used in this module after switching to theexperimentfixture/config objects. This will fail linting (unused import).
from icon4py.model.testing import (
datatest_utils as dt_utils,
definitions as test_defs,
grid_utils,
parallel_helpers,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
cscs-ci run default |
|
cscs-ci run distributed |
|
cscs-ci run default |
|
cscs-ci run distributed |
msimberg
left a comment
There was a problem hiding this comment.
Looks good to me, but something is not right with the standalone driver test: https://gitlab.com/cscs-ci/ci-testing/webhook-ci/mirrors/5125340235196978/2255149825504669/-/jobs/14888741985#L635. Is it possibly still incorrectly running more than one time step, hence the diff is still very large?
|
cscs-ci run default |
|
cscs-ci run distributed |
|
Mandatory Tests Please make sure you run these tests via comment before you merge!
Optional Tests To run benchmarks you can use:
To run tests and benchmarks with the DaCe backend you can use:
To run test levels ignored by the default test suite (mostly simple datatest for static fields computations) you can use:
For more detailed information please look at CI in the EXCLAIM universe. |
common/io: - schedule output by step count: FieldGroupIOConfig.output_interval_steps (write every N calls to store); remove the time-based path (to_delta, OutputInterval, start_time, next_output_time). Output now lands on exact step boundaries regardless of the (possibly non-integer) time step length - rename FieldMetaData.is_on_interface -> is_on_half_levels - merge to_host_data_array into to_data_array(..., to_host=...) - update the module docstring examples to the step-based config standalone_driver: - drive prognostic output from states.data.PROGNOSTIC_CF_ATTRIBUTES directly (CF names, icon_var_name lookup, is_on_half_levels); drop the parallel metadata table - rename DiagnosticInputs -> DiagnosticFields; derive them from the field factories instead of passing them into the driver constructor - add opt-in initial-conditions output (output_initial_state) - nc_title/nc_comment tidy-up; de-"boxed" docstrings - revert the main:app entry-point change back to the click() shim (the entry point fix comes with #1304) Verified: pre-commit, common io + driver unit tests, and the JW output integration datatest (gtfn_cpu) all pass.
refactor driver cli/testing interface from a single
main.main()entry point to one for testing and one dedicated to CLIalso allows for overriding config variables read from the fortran namelists such as end_date / n_time_steps
which allows for restoring the original tolerances that were altered by #1295 since the JW experiment ran for 3 time steps instead of 1