Update climacommon to 2025_03_18 #1229
Conversation
This commit introduces more structure in ClimaEarth. It splits `setup_and_run` in three functions: - `setup`, which produces a `CoupledSimulation` (hence, it was renamed to be `CoupledSimulation`, since it is just a constructor) - `run!`, which runs the simulation. This is supported by the introduction of `step!`, to take one single coupling step. This is particularly useful in debugging - `postprocess`, to make plots The advantage of this separation is that one can now construct a simulation without running it, and step a simulation one step at a time. Both of these are particularly useful in interactive work and debugging. This commit also renames `calendar_dt` to `diag_calendar_dt` to make it clearer that it refers to diagnostics.
Sbozzolo
force-pushed
the
gb/update-climacommon-to-2025_03_18
branch
from
1739919 to
b8c8d16
Compare
WalkthroughThis pull request updates several Buildkite pipeline configurations by upgrading the Changes
Possibly related PRs
Suggested Reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
NEWS.md (1)
9-9: Use consistent heading style.According to the markdownlint-cli2 rules, the heading on line 9 uses atx style (# symbols) while the document generally uses setext style headings (underlined with dashes or equals signs).
-### Split `setup_and_run` in multiple functions. PR[#1251](https://github.com/CliMA/ClimaCoupler.jl/pull/1251) +Split `setup_and_run` in multiple functions. PR[#1251](https://github.com/CliMA/ClimaCoupler.jl/pull/1251) +-----------------------------------------------------------------------------------------------🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
9-9: Heading style
Expected: setext; Actual: atx(MD003, heading-style)
experiments/ClimaEarth/run_amip.jl (1)
49-52: Consider addressing TODO comment.The comment "TODO: Remove this option?" indicates uncertainty about whether the
conservation_softfailoption should be kept. Consider making a decision about this option's future and either remove the comment or provide more context about why it might be removed.experiments/ClimaEarth/user_io/coupler_diagnostics.jl (1)
6-6: Update docstring parameter list.The docstring still shows only
diagnostics_dtas a parameter, but the function now takes an additionalcoupled_dtparameter. Update the docstring to include all parameters.- coupler_diagnostics_setup(fields, output_dir, start_date, t_start, diagnostics_dt) + coupler_diagnostics_setup(fields, output_dir, start_date, t_start, diagnostics_dt, coupled_dt)experiments/ClimaEarth/setup_run.jl (2)
101-101: Large function body alert.The body of
CoupledSimulation(config_dict::AbstractDict)is sizable. Consider extracting logic (e.g., restarts, directories setup, model initialization) into smaller helper functions to improve readability and maintainability.
638-646: SYPD (Simulated Years per Day) computation.Using a constant 365.25 for the year factor is a common approximation, though leap-year or alternative calendar considerations can be more precise. Provide a short note to future maintainers if further calendar specificity is needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.buildkite/amip/pipeline.yml(1 hunks).buildkite/benchmarks/pipeline.yml(1 hunks).buildkite/hierarchies/pipeline.yml(7 hunks).buildkite/longruns/pipeline.yml(5 hunks).buildkite/nightly/pipeline.yml(1 hunks).buildkite/pipeline.yml(1 hunks)NEWS.md(1 hunks)experiments/ClimaEarth/run_amip.jl(1 hunks)experiments/ClimaEarth/setup_run.jl(9 hunks)experiments/ClimaEarth/user_io/arg_parsing.jl(7 hunks)experiments/ClimaEarth/user_io/coupler_diagnostics.jl(2 hunks)experiments/ClimaEarth/user_io/postprocessing.jl(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`*`: # CodeRabbit Style Guide (CliMA Inspired)
Leverage CodeRabbit for code reviews aligning with CliMA's practices.
I. Key Areas for CodeRabbit:
- Style: Naming (Titl...
*: # CodeRabbit Style Guide (CliMA Inspired)Leverage CodeRabbit for code reviews aligning with CliMA's practices.
I. Key Areas for CodeRabbit:
- Style: Naming (TitleCase, lowercase_with_underscores), line length (<78), indentation (4 spaces), import order.
- DRY: Flag duplicated code; encourage modularization.
- Docstrings: Flag missing docstrings for modules, structs, functions.
- Tests: Detect missing unit tests (if configured).
- Complexity: Report on cyclomatic complexity.
II. Conventions (CodeRabbit Can Help):
- Naming: Follow CliMA/CMIP conventions. Avoid
l,O,Ias single-char vars.- Unicode: Human review needed. Acceptable chars.
III. Documentation (CodeRabbit Flags Deficiencies):
- Docstrings: Describe purpose, inputs, outputs, examples.
IV. Testing (CodeRabbit: Missing Tests):
- Unit tests required for new/modified code.
V. CodeRabbit Config:
.coderabbit.yaml: Customize language, line length, tests.VI. Workflow:
- Review CodeRabbit's suggestions; investigate, provide context, address issues, improve config.
VII. Abandoned Code: Delete or clearly mark as deprecated.
VIII. Reviewer Responsibilities: Ensure adherence to best practices.
NEWS.md
🪛 markdownlint-cli2 (0.17.2)
NEWS.md
9-9: Heading style
Expected: setext; Actual: atx
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: ci 1.11 - windows-latest
- GitHub Check: ci 1.11 - macOS-latest
- GitHub Check: ci 1.11 - ubuntu-latest
- GitHub Check: ci 1.10 - windows-latest
- GitHub Check: test (1.11)
- GitHub Check: ci 1.10 - macOS-latest
- GitHub Check: test (1.10)
- GitHub Check: ci 1.10 - ubuntu-latest
🔇 Additional comments (41)
.buildkite/benchmarks/pipeline.yml (1)
4-4: Module Version Update Verified.
Themodulesentry is updated toclimacommon/2025_03_18which aligns with the PR objective and is consistent with the version upgrade across related pipeline configurations..buildkite/amip/pipeline.yml (1)
4-4: Module Version Update Verified.
The updatedmodules: climacommon/2025_03_18entry ensures that the AMIP pipeline uses the latest climacommon version, reflecting the coordinated update across pipelines..buildkite/pipeline.yml (2)
4-4: Primary Agent Module Update Confirmed.
Themodulesfield on line 4 is updated toclimacommon/2025_03_18, ensuring that the main agent configuration aligns with the updated dependency version.
301-301: Secondary Module Field Update in Target Job Verified.
At line 301, themodulesfield for the "GPU AMIP FINE: new target amip: topo + diagedmf" job is updated toclimacommon/2025_03_18. This maintains consistency with the overall update across the CI pipelines..buildkite/nightly/pipeline.yml (1)
4-4: Module Version Update Verified for Nightly Pipeline.
The update on line 4 tomodules: climacommon/2025_03_18ensures that nightly jobs will run with the latest climacommon version, in line with the PR objectives..buildkite/longruns/pipeline.yml (1)
4-4: Module Version Update Verified for Longruns.
Themodulesfield is updated toclimacommon/2025_03_18, which is consistent with the changes applied to other pipelines, ensuring uniform dependency versions..buildkite/hierarchies/pipeline.yml (7)
4-4: Module Version Update in Default Agents:
Themodulesfield on line 4 is updated toclimacommon/2025_03_18, aligning the default agent configuration with the latest release. This change supports the intended compatibility with Julia 1.11.4 and CUDA 5.7.
33-33: Module Version Update for Init Clima Environment:
On line 33, themodulesfield in theinit climastep is updated toclimacommon/2025_03_18. This uniform update ensures that the initialization process uses the correct module version.
55-55: Module Version Update in GPU ClimaCoupler dry HS Step:
The update on line 55 sets the module version for the "Clima: GPU ClimaCoupler dry HS" step toclimacommon/2025_03_18, ensuring consistency and compatibility with the newer dependencies.
67-67: Module Version Update in GPU ClimaCoupler moist HS Step:
Line 67 has been updated to useclimacommon/2025_03_18for the moist Held Suárez simulation. This change maintains uniformity across the GPU-run configurations.
79-79: Cloudless Aquaplanet Module Update:
Themodulesentry on line 79 is updated toclimacommon/2025_03_18in the "Clima: GPU ClimaCoupler Cloudless Aquaplanet" step, aligning it with the overall module upgrade.
91-91: Cloudy Aquaplanet Module Update:
On line 91, the configuration is updated to useclimacommon/2025_03_18, ensuring that the "Clima: GPU ClimaCoupler Cloudy Aquaplanet" step leverages the latest module version.
103-103: Cloudy Slabplanet Module Update:
Line 103 now specifiesclimacommon/2025_03_18for the "Clima: GPU ClimaCoupler Cloudy Slabplanet" step, making sure that all the GPU steps uniformly reference the updated module version.NEWS.md (1)
11-22: Well-documented restructuring - good job!The documentation clearly explains how
setup_and_runwas split into three separate functions and introduces the newstep!function. The renaming ofcalendar_dttodiagnostics_dtis also well-explained, improving clarity throughout the codebase.experiments/ClimaEarth/run_amip.jl (1)
46-48: Good refactoring to object-oriented approach.The change from a direct function call to creating a
CoupledSimulationinstance and callingrun!follows good object-oriented principles, making the code more modular and maintainable.experiments/ClimaEarth/user_io/coupler_diagnostics.jl (1)
14-14: Parameter renaming and function upgrade well implemented.The changes correctly implement the renaming from
calendar_dttodiagnostics_dtand add the newcoupled_dtparameter, enhancing the function's capabilities. The update to theDiagnosticsHandlerconstructor with the newdtparameter is a good improvement.Also applies to: 17-17, 51-51
experiments/ClimaEarth/user_io/arg_parsing.jl (4)
102-102: Variable renaming correctly implemented in get_coupler_args.The renaming from
calendar_dttodiagnostics_dthas been correctly implemented in the function call and return value, maintaining consistency with the changes in other files.Also applies to: 137-137
165-165: Simplified return value in get_atmos_args.The removal of
output_default_diagnosticsfrom the return value ofget_atmos_argshelps clean up the code by removing unused parameters. This is a good practice.
189-189: Consistent variable renaming in get_diag_period.The renaming from
calendar_dttodiagnostics_dtis consistently applied throughout theget_diag_periodfunction, including documentation, variable assignments, and the return statement.Also applies to: 197-197, 201-201, 205-205, 209-209, 211-211
273-274: Consistent variable renaming in add_extra_diagnostics!.The renaming from
calendar_dttodiagnostics_dtis correctly implemented in theadd_extra_diagnostics!function, both in the documentation and code.Also applies to: 285-286, 288-288, 298-298
experiments/ClimaEarth/user_io/postprocessing.jl (2)
15-15: Confirm only extractingconservation_softfail.Previously, other variables like
t_endoroutput_default_diagnosticsmay have been extracted frompostprocessing_vars. Ensure there are no downstream dependencies on those missing variables to avoid runtime issues.
31-41: Validate data presence and usage of 84600 seconds.
- If
"pfull"is present but no valid time data exists, callingtimes[end]may raise an error. Consider adding checks to avoid out-of-bounds errors on an empty array.- 84600 seconds is slightly less than the typical 86400 seconds in a day. Confirm that this difference is intentional and does not cause off-by-one issues in determining the spin-up threshold.
experiments/ClimaEarth/setup_run.jl (19)
38-38: ImportingCoupledSimulation.This import from the
Interfacermodule looks correct. It makes theCoupledSimulationsymbol readily available in this file.
85-94: Docstring clarity.The docstring explaining
CoupledSimulationconstructors is concise and helpful. It covers the purpose and usage adequately.
97-97: Forwarding the config dictionary.Forwarding directly to
CoupledSimulation(config_dict)is straightforward and helps maintain the single source of initialization logic.
127-128: Use ofdiagnostics_dt.Exposing and storing
diagnostics_dtensures consistent scheduling of diagnostics across the simulation. This addition cleanly unifies the naming.
136-136:use_coupler_diagnosticsflag.Including this boolean at the configuration level is a clear and convenient way to toggle coupler diagnostics.
163-163: Check consistency ofdt_rad.
dt_radis set fromatmos_config_dict. Confirm that this radiation timestep aligns with the core coupler or model timesteps to avoid unintentional phase offsets.
499-501: Setting up coupler diagnostics.The call to
coupler_diagnostics_setup(...)correctly includes the necessary parameters for logging and scheduling. This is good for consolidated data handling.
514-514: Typed construction of theCoupledSimulationinstance.Using
CoupledSimulation{FT}(ensures strong type safety for floating-point operations in the coupled model.
528-528: Configuration ofdiags_handler.Appending the diagnostic handler to the constructor call is consistent with the overall design.
597-598: Return statement.Returning
csconcludes the constructor properly, ensuring the fully initializedCoupledSimulationis provided to the caller.
600-616:run!function overview.
- Docstring is well-structured, indicating usage of
precompile.- Precompilation step is practical for HPC runs, ensuring accurate timing.
- Consider a short note if
tspan[end]is extremely small—precompile might consume most of the runtime in such edge cases.
629-634: Simulation loop withwhile cs.t[] <= cs.tspan[end].Entering the loop until
cs.t[] <= cs.tspan[end]clarifies the inclusive end condition. Ensure that any fractional leftover time is either accepted or handled explicitly if partial steps are needed (though not typically done in production climate runs).
648-654: Artifacts logging.Storing simulation performance metrics in
sypd.txtandwalltime_per_step.txtensures easy integration with CI monitoring or downstream analytics.
662-664: Closing diagnostic writers.Proactively closing all outputs at the end of the run is good practice for freeing resources and flushing all data to disk.
668-697:postprocessfunction.
- Good usage of
ClimaComms.iamroot(cs.comms_ctx)to restrict postprocessing to the root rank.- Passing a minimal set of variables (
; conservation_softfail) keeps the function call succinct.- Consider logging any failures or partial steps in the future for robust debugging.
707-711:setup_and_run(config_file)entry point.A concise wrapper that creates a simulation and runs it. Straightforward and clear—the function name conveys the intent perfectly.
713-717:setup_and_run(config_dict)variant.Similarly concise for users who already have a configuration dictionary in hand, which avoids reading from an external file.
719-727: Docstring forstep!(cs::CoupledSimulation).The docstring explicitly clarifies that one coupling step can contain multiple steps in specific component models. This is accurate and well-explained.
728-784:step!method logic.
- Time is advanced by
Δt_cplin a well-structured way.- The series of
update_*calls and flux calculations is logically ordered—surfaces are updated, fluxes recalculated, and atmosphere states are synced.- The final callback for diagnostics is triggered with clear scheduling.
This meticulous approach ensures consistent surface-atmosphere coupling at each step.
| about component models, diagnostics, timestepping, output directories, etc | ||
| needed to run a coupled simulation. | ||
| """ | ||
| function CoupledSimulation(config_file = joinpath(pkgdir(ClimaCoupler), "config/ci_configs/amip_default.yml")) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding file-not-found handling.
If the config_file is missing or unreadable, the code may fail unexpectedly. Adding a simple check or try-catch block would improve resilience:
function CoupledSimulation(config_file = joinpath(pkgdir(ClimaCoupler), "config/ci_configs/amip_default.yml"))
+ if !isfile(config_file)
+ error("Config file not found: $config_file")
+ end
config_dict = get_coupler_config_dict(config_file)
return CoupledSimulation(config_dict)
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function CoupledSimulation(config_file = joinpath(pkgdir(ClimaCoupler), "config/ci_configs/amip_default.yml")) | |
| function CoupledSimulation(config_file = joinpath(pkgdir(ClimaCoupler), "config/ci_configs/amip_default.yml")) | |
| if !isfile(config_file) | |
| error("Config file not found: $config_file") | |
| end | |
| config_dict = get_coupler_config_dict(config_file) | |
| return CoupledSimulation(config_dict) | |
| end |
🤖 Beep boop. I am GabrieleBOT. 🤖
I received an update so that I can inform you directly of the changes (but feel free to check the release notes).
The most recent version of climacommon uses the newly released Julia 1.11.4.
This version is needed to use CUDA 5.7, so I would recommend updating.
Summary by CodeRabbit
Chores
New Features
Refactor