Skip to content

Add the ability to use NetCDF#125

Merged
simone-silvestri merged 5 commits into
mainfrom
ss/allow-netcdf
Mar 25, 2026
Merged

Add the ability to use NetCDF#125
simone-silvestri merged 5 commits into
mainfrom
ss/allow-netcdf

Conversation

@simone-silvestri
Copy link
Copy Markdown
Collaborator

No description provided.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds optional NetCDF (NCDatasets-backed) output support to ClimaSeaIce by wiring in a Julia package extension, plus coverage in CI via a new NetCDF-focused test group.

Changes:

  • Add ClimaSeaIceNCDatasetsExt extension to provide NetCDF output attributes for SeaIceModel.
  • Add a NetCDFWriter integration test and hook it into runtests.jl under a new TEST_GROUP.
  • Register NCDatasets as a weak dependency/extension trigger, bump package version, and add a dedicated CI job for the new test group.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
ext/ClimaSeaIceNCDatasetsExt.jl New extension module defining default NetCDF output attributes for sea-ice fields (and velocities).
test/test_netcdf_writer.jl New test that runs a short simulation and validates NetCDF output contents.
test/runtests.jl Adds a netcdf test group include.
Project.toml Bumps version; adds NCDatasets weakdep/extension and adds it to test target.
.github/workflows/ci.yml Adds a CI job to run only the netcdf test group.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ext/ClimaSeaIceNCDatasetsExt.jl Outdated
Comment thread ext/ClimaSeaIceNCDatasetsExt.jl Outdated
Comment on lines +32 to +35
function OCNE.default_output_attributes(model::SeaIceModel)
velocity_attrs = default_horizontal_velocity_attributes(model.grid)
tracer_attrs = default_sea_ice_attributes()
return merge(velocity_attrs, tracer_attrs)
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base.get_extension(Oceananigans, :OceananigansNCDatasetsExt) can return nothing. Defining function OCNE.default_output_attributes(...) will then error at extension load time (Nothing has no field default_output_attributes). Please either (a) extend a binding defined in the parent Oceananigans module (preferred), or (b) guard against isnothing(OCNE) before defining the method / provide a fallback.

Suggested change
function OCNE.default_output_attributes(model::SeaIceModel)
velocity_attrs = default_horizontal_velocity_attributes(model.grid)
tracer_attrs = default_sea_ice_attributes()
return merge(velocity_attrs, tracer_attrs)
if !isnothing(OCNE)
function OCNE.default_output_attributes(model::SeaIceModel)
velocity_attrs = default_horizontal_velocity_attributes(model.grid)
tracer_attrs = default_sea_ice_attributes()
return merge(velocity_attrs, tracer_attrs)
end

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Comment thread ext/ClimaSeaIceNCDatasetsExt.jl Outdated

default_horizontal_velocity_attributes(::OrthogonalSphericalShellGrid) = Dict(
"u" => Dict("long_name" => "Velocity in the i-direction (+ = increasing i).", "units" => "m/s"),
"v" => Dict("long_name" => "Velocity in the j-direction (+ = increasing j).", "units" => "m/s"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent

@simone-silvestri simone-silvestri merged commit 20a58e6 into main Mar 25, 2026
8 checks passed
@simone-silvestri simone-silvestri deleted the ss/allow-netcdf branch March 25, 2026 21:31
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.

3 participants