Skip to content

Conversation

@nefrathenrici
Copy link
Member

@nefrathenrici nefrathenrici commented Sep 2, 2025

Purpose

Adds a new AtmosSimulation constructor, closes #3894.

The current AtmosConfig/parsed_args system is monolithic and hard to use programmatically. This PR introduces a more composable constructor that makes it easier to create simulations and reduces coupling between configuration parsing and simulation setup. The core changes are in src/simulation/AtmosSimulations.jl

Content

Core Changes

  • New AtmosSimulation{FT} constructor: Added a keyword-based constructor that doesn't require AtmosConfig/parsed_args, allowing programmatic simulation creation with explicit parameters
  • Callback system refactoring: Modularized callback creation into reusable functions:
    • default_model_callbacks: Creates callbacks from model components
    • common_callbacks: Standard simulation callbacks (progress logging, NaN checking, checkpointing, etc.)
    • Individual callback builders in get_callbacks.jl
  • Diagnostics setup: Added setup_diagnostics_and_writers function to handle diagnostics initialization for both old and new constructors
  • Updated struct defaults in types.jl to match YAML config defaults:

Utilities

  • Added utility functions: convert_time_args, handle_restart, extract_diagnostic_periods, validate_checkpoint_diagnostics_consistency, parse_checkpoint_frequency
  • Separated parsed_args and sim_info from functions to enable reuse: args_integrator, get_jacobian, ode_configuration, get_state_restart, get_diagnostics, get_callbacks, build_cache
  • Added test for restart functionality with new constructor test/restart_AtmosSimulation.jl

Miscellaneous

  • Removed _DEFAULT_ATMOS_MODEL_KWARGS constant - defaults now handled by struct definitions
  • Removed some logging statements for cleaner output
  • Updated core_default_diagnostics to require topography parameter explicitly

TODOs

  • Add unit tests for new constructor

Future Work

  • Unify callback system - get_callbacks and common_callbacks have some redundancy
  • Add support for missing callbacks (SCM forcing, etc.)
  • Obtain tracers from model info automatically
  • Validate callback frequencies with dt
  • Add netcdf_interpolation_num_points and netcdf_output_at_levels to new constructor
  • Ensure get_simulation retains all desired logging
  • Add "verbose" option with all logging from get_simulation
  • Add more defaults for AtmosModel subtypes
  • Improve simulation validation with informative errors (currently nonsensical simulations fail later with confusing errors)
  • Catalog config options possible with get_simulation but not with new AtmosSimulation constructor
  • Improve remaining jacobian configuration
  • Improve callback configuration, further simplify builders

@nefrathenrici nefrathenrici force-pushed the ne/refactor branch 2 times, most recently from da22283 to 57c98fe Compare December 19, 2025 04:52
@nefrathenrici nefrathenrici marked this pull request as ready for review December 19, 2025 04:53
@nefrathenrici nefrathenrici force-pushed the ne/refactor branch 3 times, most recently from 43604b6 to db90bdd Compare December 19, 2025 23:03
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.

AtmosSimulation Constructor Refactor

2 participants