What time attribute should Oceananigans use for NetCDF?

[ali-ramadhan]

Oceanangans.jl v0.51.0 changed the NetCDF time attribute from

Dict("longname" => "Time", "units" => "s")

to

Dict("longname" => "Time", "units" => "seconds")  # seemed clearer to me.

so now some packages such as xarray decode the time dimension as a timedelta instead of just a plain floating-point number.

Old behavior can be reproduced via the decode_times=False kwarg for xarray.open_dataset.

I think this is a good change but the unintended consequence surprised some users (cc @suyashbire1) so I'm opening this issue to see if we indeed want this. If we don't then we shouldn't add a time attribute (and provide a way for users to specify one).

@tomchor Maybe you have some thoughts since you've used NetCDF/xarray/Oceananigans.jl quite extensively.

[tomchor]

I noticed this change last week but I thought it was intentional. I don't have a strong opinion here.

If we're trying to cater to more inexperienced users (of which Oceananigans will probably attract a lot of, since it's always portrayed as user-friendly) then it might make sense to try and guard against this somehow (since the errors you get when trying to do stuff with timedeltas that can't be done aren't very useful).

Otherwise I'd say it's up to the user to deal with this, since on the Oceananigans end the change was (imho) for the better.

[ali-ramadhan]

Sounds like the consensus is to keep the new time attribute then. So I'll close the issue unless anyone has a strong opinion.

What do other models do? If every model outputs time in "seconds" then maybe what Oceananigans.jl does is not too surprising.

[glwagner]

But Oceananigans doesn't really output time in "seconds", does it? The time unit is arbitrary, or depends on user inputs.

[ali-ramadhan]

Right, another reason to allow the user to specify the time attribute when using NetCDFOutputWriter.

The default IncompressibleModel uses seconds so that could be an argument for "seconds" being the default for NetCDFOutputWriter.

But right now it's not configurable by the user so I'll keep this issue open until this is resolved.

[glwagner]

The default IncompressibleModel uses seconds

Ah, because of the default buoyancy=SeawaterBuoyancy?

[glwagner]

Mmm yeah, I think settable units with a sensible default makes sense. There are a number of examples that are non-dimensional, so you might say that using NetCDFOutputWriter with those right now leads to a bug (misattribution of the units of the time dimension).

Can spatial units be set?

[tomchor]

Are packages like Unitful.jl not mature enough to be used for these purposes? I understand this is a pretty extreme solution for this problem specifically, but It would be pretty cool if Oceananigans could understand units.

[ali-ramadhan]

Can spatial units be set?

Spatial attributes can be set but Oceananigans.jl assumes meters right now so more unit misattribution. If the time units can be set, then we should be able to set the spatial units.

Are packages like Unitful.jl not mature enough to be used for these purposes? I understand this is a pretty extreme solution for this problem specifically, but It would be pretty cool if Oceananigans could understand units.

I think it's mature enough but not I guess we're not sure where to use it (we discussed a bit in #1116). Would be perfect for specifying these kinds of units as long as it's not a heavy dependency. For other simulations, e.g. icy moons, it's nice to use rotation periods.

Another place where units are assumed is prettytime so the Simulation always thinks your units are SI.

[glwagner]

That might work. I think using that might require a new quasi-global property like grid.units. Users would have to write their scripts with a bit more intent because they'd have to specify the unit system when constructing the grid.

[ali-ramadhan]

Ah that would be a good place for it. Maybe grid.units and clock.units are the only new properties, and everything is then inferred from these?

[tomchor]

Users would have to write their scripts with a bit more intent because they'd have to specify the unit system when constructing the grid.

I mean, we can always assume the units are SI unless the user says otherwise, right? I think 90+% of Oceananigans runs are in SI, so it would make sense to assume that while making it easy for anyone to overwrite that with a keyword. That way 90+% of scripts wouldn't need any additional lines of code.

[ali-ramadhan]

Yeah if the default model assumes SI, then seems reasonable for the default metadata to assume SI.

Unitful.jl looks pretty lightweight so might not be so bad to use it if we think it'll be useful: https://github.com/PainterQubits/Unitful.jl/blob/master/Project.toml

[glwagner]

Agree that SI default makes sense for a code called "Oceananigans".

Note that some of the examples are non-dimensional, so an SI default will indeed add code to those examples --- which are likely some of the most-read scripts in the Oceananigans-verse. I also think for accessibility it could make sense to prioritize ease-of-use in a classroom context, where non-dimensional runs are the norm.

[ali-ramadhan]

Yeah either way we definitely want to make it easy to switch between dimensional and non-dimensional models so there may be some overlap with #1427.

Was thinking for a complete specification we might need a unit for time, length, mass, and one for each tracer. Seems like we might as well go all the way and not just add units for length and time.

[glwagner]

Any movement here? Otherwise we might want to close it.

[tomchor]

Personally, I think things are working well as they are now. We're assuming time is in seconds, but also that positions are in meters and so on. We could create a flat in NetCDFWriter that switches from metric no nondimensional, but unless other users seem bothered by this, I say we just close the issue for now.

Ideally we'd solve this issue for Oceananigans as a whole by using Unitful and such, but that's probably a very big effort...

[glwagner]

Ok, lets shelve it for now. Thanks!