E3SM-Project / polaris

Testing and analysis for OMEGA, MPAS-Ocean, MALI and MPAS-Seaice
BSD 3-Clause "New" or "Revised" License
6 stars 13 forks source link

Assume mesh variables only in mesh file #97

Closed cbegeman closed 1 year ago

cbegeman commented 1 year ago

For OMEGA, mesh variables will be located in a separate netcdf file from the output. This PR always loads mesh variables from the mesh netcdf (usually mesh.nc or culled_mesh.nc). Some of my changes from the init netcdf to the mesh netcdf file are unnecessary but I think it improves code clarity.

Checklist

cbegeman commented 1 year ago

Testing

This PR passes PR and cosine_bell suites on chrys with intel-mpi

xylar commented 1 year ago

I'm curious about the various ds.close() calls. I assume that was just to tidy things up by being explicit about the files being closed? The files should always be closed automatically when the ds object gets cleaned up at the end of the function.

xylar commented 1 year ago

The slightly preferred way to handle scope for an xarray dataset that isn't needed for the full scope of the function or method is something like:

with xr.open_dataset('mesh.nc') as ds_mesh:
    xCell = ds_mesh.xCell.values
    ...

In this case, ds_mesh.close() is automatically called at the end of the with statement and the scope of ds_mesh is clear. It is important not to refer to ds_mesh or any of its data arrays outside of that scope.

I don't think we likely want to do that retroactively because it changes a lot of whitespace but I would consider it if it's important to you to have an indication of the scope of ds, ds_mesh, etc. I have a slight preference to not having explicit ds.close() calls as that isn't a common practice but could accept them, too.

cbegeman commented 1 year ago

I'm curious about the various ds.close() calls. I assume that was just to tidy things up by being explicit about the files being closed? The files should always be closed automatically when the ds object gets cleaned up at the end of the function.

I'm happy to remove them if they're not necessary

cbegeman commented 1 year ago

@xylar ds.close gone now. Ready for re-review.

xylar commented 1 year ago

Thanks for engaging with my nit-picking on ds.close().