Output writer "diagnostic dependencies"

[glwagner]

This PR introduces the concept of an output writer "diagnostic dependency": a diagnostic which must be added to simulation.diagnostics in order for output to be correct.

If a type of output has a "diagnostic dependency", it must define the function add_dependency!(diagnostics, output), which will add any appropriate diagnostics to the ordered dictionary diagnostics.

Currently we only have one type of output that requires a diagnostic, which is the WindowedTimeAverage. For WindowedTimeAverage, add_dependency! is

add_dependency!(diags, wta::WindowedTimeAverage) = !(wta ∈ values(diags)) && push!(diags, wta)

(As a side note, we could require that all values of diagnostics are unique, which would obviate the check above.)

[glwagner]

I couldn't get tests to work for the NetCDFOutputWriter. @ali-ramadhan can you take a look at

Oceananigans.jl/test/test_simulations.jl

Lines 67 to 75 in ede8021

	#attributes = Dict(:time_average => Dict("longname" => "A time average",
	# "units" => "arbitrary")windowed_time_average_attributes)
	#
	#dimensions = Dict(:time_average => ("xF", "yC", "zC"))
	#
	#netcdf_output_writer = NetCDFOutputWriter(model, output, time_interval=4.0, filename="test.nc",
	# output_attributes=attributes, dimensions=dimensions)
	#
	#@test dependencies_added_correctly!(model, windowed_time_average, netcdf_output_writer)

[codecov]

Codecov Report

Merging #886 into master will increase coverage by 0.67%.
The diff coverage is 97.77%.

@@            Coverage Diff             @@
##           master     #886      +/-   ##
==========================================
+ Coverage   70.99%   71.67%   +0.67%     
==========================================
  Files         188      188              
  Lines        5230     5433     +203     
==========================================
+ Hits         3713     3894     +181     
- Misses       1517     1539      +22     

Impacted Files	Coverage Δ
src/Simulations/run.jl	93.61% <83.33%> (-1.63%)	⬇️
test/test_output_writers.jl	93.15% <100.00%> (+0.36%)	⬆️
test/test_simulations.jl	100.00% <100.00%> (ø)
src/Fields/field.jl	66.66% <0.00%> (+1.04%)	⬆️
src/AbstractOperations/computations.jl	72.34% <0.00%> (+1.28%)	⬆️
src/Grids/grid_utils.jl	90.50% <0.00%> (+2.33%)	⬆️
src/OutputWriters/netcdf_output_writer.jl	83.33% <0.00%> (+2.81%)	⬆️
src/Diagnostics/average.jl	97.29% <0.00%> (+5.99%)	⬆️
src/Grids/Grids.jl	78.57% <0.00%> (+7.14%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fbbb5f8...d326819. Read the comment docs.

[ali-ramadhan]

Fixed the NetCDFOutputWriter test for dependencies_added_correctly! so it passes now. I think since it goes through a C library, NetCDFOutputWriter needs strings as keys, while it was using symbols. Annoying, but probably a NetCDF limitation.

Also, simulations are tested before output writers so I'm slightly concerned that we are relying on output writers before they are tested so there's no guarantee they even work. I.e. the dependencies_added_correctly! tests should go into test_output_writers.jl.

[ali-ramadhan]

I would probably write this as

wta ∈ values(diags) || push!(diags, wta)

but maybe it's a bit more obfuscated.

[glwagner]

Oh that's nice.

[glwagner]

@ali-ramadhan I'll move the tests to test_output_writers.jl.

[glwagner]

@ali-ramadhan can you restart gitlab?