Add methods to write a field directly to NetCDF#4730
Conversation
|
Excited for this! |
There was a problem hiding this comment.
Pull Request Overview
This PR adds the capability to write Oceananigans Field objects directly to NetCDF files by extending the defVar function. This decouples NetCDF writing from simulation contexts and enables writing fields from different grids to the same file.
Key changes:
- Extended
defVarto handleAbstractField,AbstractOperation, andReductionobjects - Moved dimension name generation utilities from extension to main module
- Added validation and creation of field dimensions in NetCDF datasets
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/OutputWriters/netcdf_writer.jl | Moved dimension name generation functions from extension module to main module |
| ext/OceananigansNCDatasetsExt.jl | Extended defVar for fields and added dimension validation/creation functions |
| test/test_netcdf_writer.jl | Added comprehensive tests for single field writing, dimension validation, and multiple grid support |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
ali-ramadhan
left a comment
There was a problem hiding this comment.
Looks good to me! I think the PR could just do with some more testing:
- I would run the newly added tests on both CPU and GPU, especially to ensure no scalar indexing occurs.
- I would also run the newly added tests on
LatitudeLongitudeGridand immersed grids (both immersed rectilinear and immersed lat-lon).
This feature also allows for fields with different underlying grids, or windowed different, and even fields from two completely different simulations, to live in the same NetCDF file. All that is needed is to pass a different
dimension_name_generatorfor each.
That's awesome! Just to confirm: for a user to utilize this feature, I assume it's the job of each dimension_name_generator to generate unique dimension names? Actually reading through the tests has answered my question (yes)!
Good points! I actually was supposed to add GPU from the start (since those tests are on an
Yeah, I think this is a limitation of NetCDF unfortunately. Variables that use the same dimension name must have the same shape. So different shapes require different dimension names :( |
… into tc/field-to-netcdf
| minimal_location_string(::RectilinearGrid, LX, LY, LZ, ::Val{:x}) = loc2letter(LX, false) | ||
| minimal_location_string(::RectilinearGrid, LX, LY, LZ, ::Val{:y}) = loc2letter(LY, false) | ||
|
|
||
| minimal_dim_name(var_name, grid, LX, LY, LZ, dim) = |
There was a problem hiding this comment.
What was this line supposed to do? It's defining a function which returns a function, I doubt this is what you meant. Also, this is untested: https://app.codecov.io/gh/CliMA/Oceananigans.jl/blob/main/src%2FOutputWriters%2Fnetcdf_writer.jl#L47
This PR does a bit of reorganizing but mainly it makes it possible to write a
Fielddirectly into a NetCDF file. The motivations are:OpenBoundaryConditions withschemesand use Tuples inboundary_mass_fluxes#4687 is merged, which supports non-immersed grid reconstruction)dimension_name_generatorfor each.Here's a quick example:
For now I decided to extend the existing
defVarin order to avoid creating new functions, but we can definitely also create a new function specifically for that (to_netcdf(field::AbstractField)?).