Skip to content

Fix error reading FieldTimeSeries from NetCDF#5684

Open
tomchor wants to merge 6 commits into
mainfrom
tc/netcdf-eval-fix
Open

Fix error reading FieldTimeSeries from NetCDF#5684
tomchor wants to merge 6 commits into
mainfrom
tc/netcdf-eval-fix

Conversation

@tomchor

@tomchor tomchor commented Jun 14, 2026

Copy link
Copy Markdown
Member

Reading a FieldTimeSeries from a NetCDF file is throwing an error when Oceananigans isn't in the main scope.
This is because strings were eval'd in the extension's scope, where the Oceananigans name isn't bound.

MWE:

import Oceananigans as OC
import NCDatasets

grid  = OC.RectilinearGrid(size=(4, 4, 4), extent=(1, 1, 1))
model = OC.NonhydrostaticModel(grid; tracers=:c)

sim = OC.Simulation(model; Δt=1, stop_iteration=1)
sim.output_writers[:fields] = OC.NetCDFWriter(model, model.tracers;
    filename="mwe_grid.nc", schedule=OC.IterationInterval(1), overwrite_existing=true)
OC.run!(sim)

OC.FieldTimeSeries("mwe_grid.nc", "c")   # throws

@tomchor tomchor changed the title Fix UndefVarError reading FieldTimeSeries from NetCDF (0.110.2) Fix error reading FieldTimeSeries from NetCDF Jun 14, 2026
Guards against the UndefVarError regression where grid-reconstruction
strings serialized as e.g. "Oceananigans.Grids.Periodic" failed to
resolve in the extension module.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@tomchor tomchor changed the title (0.110.2) Fix error reading FieldTimeSeries from NetCDF Fix error reading FieldTimeSeries from NetCDF Jun 15, 2026
Evaluating in the Oceananigans top-level module broke values that
serialize to bare, unqualified names living in submodules and not
re-exported at top level (e.g. "CenterImmersedCondition()"), surfacing
as `UndefVarError: CenterImmersedCondition not defined in Oceananigans`
when reconstructing an immersed-boundary grid.

Evaluate in the extension module instead: it imports those bare names,
and the `import Oceananigans` added earlier binds `Oceananigans` so
fully-qualified names (e.g. "Oceananigans.Grids.Periodic") also resolve.
The union of both namespaces is what covers every serialized form.

Extend the regression test to cover the bare-name forms too.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🐞 Even a perfect program still has bugs extensions 🧬

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant