nonadvective_cells_removed attribute is always 'yes'

[aekiss]

It seems to me there's nowhere in the code that sets the nonadvective_cells_removed attribute to anything other than 'yes'.

Should the default (initialization) be 'no '?

[micaeljtoliveira]

@aekiss Good point!

I think we should make sure all the parameters are set when generating the topography (i.e., when calling gen_topo), but I would not set them when reading the file if they are not already present. This is because I don't think we should make any assumptions about topographies generated with an older version of the tools or with other tools. Does this seems reasonable?

[micaeljtoliveira]

The only exception would be the grid type, as one does not know it during the call to gen_topo. In that case, maybe we should have an "undertermined" grid type to make this explicit?

[aekiss]

If we interpret nonadvective_cells_removed as indicating whether fix_nonadvective has been applied, then a default of 'no ' makes sense, since the attribute will only be absent if fix_nonadvective has never been applied.

The value of nonadvective_cells_removed is never actually used for anything, it's just metadata for the user.

[micaeljtoliveira]

@aekiss I've looked at the code and realised that actually the initialisation of nonadvective_cells_removed to yes is overwritten when trying to read the attribute from the file and it is not present (looks like the netcdf library simply sets it to an empty string).

This is easy to fix, and set it to no but then I've got a different question. I've played a bit with the tools and it seems that now the fix_non_advective command fails unless the grid type has previously been set to B. Shouldn't we instead let the user run it if the grid type has not been set at all? What do you think?

[aekiss]

I've played a bit with the tools and it seems that now the fix_non_advective command fails unless the grid type has previously been set to B. Shouldn't we instead let the user run it if the grid type has not been set at all? What do you think?

I'd intended grid_type to default to B if absent from the file, and this is how it works when I try it, so I'm surprised it behaves differently for you. Maybe it depends on the version of netcdf? I'm using

$ ldd topogtools | grep cdf
	libnetcdff.so.7 => /g/data/hh5/public/apps/miniconda3/envs/analysis3-24.07/lib/libnetcdff.so.7 (0x00007f2c5e79c000)
	libnetcdf.so.19 => /g/data/hh5/public/apps/miniconda3/envs/analysis3-24.07/lib/./libnetcdf.so.19 (0x00007f2c5db04000)
	libpnetcdf.so.5 => /g/data/hh5/public/apps/miniconda3/envs/analysis3-24.07/lib/././libpnetcdf.so.5 (0x00007f2c5c847000)

[micaeljtoliveira]

@aekiss Sorry, I was not very clear. What I meant is: do we really need to assume the grid to be of "B" type in order to allow the user to run fix_nonadvective? Shouldn't we instead set the grid to "B" after executing the command? As it is, one always sets the grid type to "B" when not present in the file, even when executing a command that is in theory grid type agnostic. For example, if I run min_max_depth on a topography that has no grid type defined, I end up with a topography file that says "grip_type = B". Is this the intended behavior?

Maybe my question is actually a more general one: what does it mean when one encounters a grid type = "B" in the netcdf file? Does it mean that the user explicitly set it to "B", or that some command was run that only makes sense for a "B" grid?