Oceananigans.jl: Use dictionaries rather than arrays to store output writers and diagnostics?

[glwagner]

It may be useful to require the members of the respective collections to have names.

[ali-ramadhan]

I like this idea. Although I've never had to access an output writer after pushing it to model.output_writers it would be way better to refer to them as model.output_writers[:profile_writer] instead of model.output_writers[2].

[glwagner]

The advantage is clearer for diagnostics: with a dictionary (or other named/associative collection) it will presumably be easier to keep track of the elements of model.diagnostics for the purpose of logging/printing, or saving their values... ?

[glwagner]

Edit: I think we should use OrderedDicts for this. This will preserve the insertion order and thus guarantees the order of execution of the diagnostics, which could be utilized in some cases if interdependencies exist between diagnostics.

[ali-ramadhan]

Looks like OrderedDict is part of DataStructures.jl so we'd be adding a new dependency, although in this case it seems like a good idea (and it's a solid package as it used to be part of Base).

Order is definitely important so it would be nice to have the option of iterating over diagnostics and accessing them by name if needed.

It would be good to have two ways of adding diagnostics or output writers:

1. model.diagnostics[:havg_u] = diag if you want to give it a name.
2. push!(model.diagnostics, diag) when you don't care about giving it a name. Or when you want to append! a bunch of diagnostics without worrying about names.

[glwagner]

I think for option 2 the best option might be to simply define a new function (add_diagnostic!(model, diag)?) which assigns the diagnostic(s) with some default name (perhaps just the index number).

[ali-ramadhan]

Yeah. or :diagnostic_$idx for prettier printing/logging.

[glwagner]

We could also extend push! for objects of type OrderedDict{Symbol, Diagnostic}. Extending push! here for this Oceananigans-specific type would motivate keeping the Diagnostic abstract type, since we can ensure that this slightly hack-y extension of push! doesn't mess anything else up in an unexpected way. And a serious bonus is that it doesn't break the API.