Use descriptive_name for filenames and errors

src/diagnostics/Diagnostics.jl

@@ -43,6 +43,7 @@
 #   - core_diagnostics.jl
 #   - default_diagnostics.jl (which defines all the higher-level interfaces and defaults)
 #   - reduction_identities.jl
+#   - diagnostic_utils.jl
 
 """
     DiagnosticVariable
@@ -138,7 +139,7 @@ function add_diagnostic_variable!(;
     long_name,
     units,
     comments,
-    compute_from_integrator
+    compute_from_integrator,
 )
     haskey(ALL_DIAGNOSTICS, short_name) &&
         error("diagnostic $short_name already defined")
@@ -148,7 +149,7 @@ function add_diagnostic_variable!(;
         long_name,
         units,
         comments,
-        compute_from_integrator
+        compute_from_integrator,
     )
 end
 
@@ -160,6 +161,9 @@ include("default_diagnostics.jl")
 
 # ScheduledDiagnostics
 
+include("diagnostics_utils.jl")
+
+
 # NOTE: The definitions of ScheduledDiagnosticTime and ScheduledDiagnosticIterations are
 # nearly identical except for the fact that one is assumed to use units of seconds the other
 # units of integration steps. However, we allow for this little repetition of code to avoid
@@ -264,16 +268,18 @@ struct ScheduledDiagnosticIterations{T1, T2, OW, F1, F2, PO}
     )
 
         # We provide an inner constructor to enforce some constraints
+        descriptive_name =
+            get_descriptive_name(variable, output_every, reduction_time_func)
 
         output_every % compute_every == 0 || error(
-            "output_every should be multiple of compute_every for variable $(variable.long_name)",
+            "output_every should be multiple of compute_every for diagnostic $(descriptive_name)",
         )
 
         isa_reduction = !isnothing(reduction_time_func)
 
         # If it is not a reduction, we compute only when we output
         if !isa_reduction && compute_every != output_every
-            @warn "output_every != compute_every for $(variable.long_name), changing compute_every to match"
+            @warn "output_every != compute_every for $(descriptive_name), changing compute_every to match"
             compute_every = output_every
         end
 
@@ -382,20 +388,26 @@ struct ScheduledDiagnosticTime{T1, T2, OW, F1, F2, PO}
     )
 
         # We provide an inner constructor to enforce some constraints
+        descriptive_name = get_descriptive_name(
+            variable,
+            output_every,
+            reduction_time_func;
+            units_are_seconds = false,
+        )
 
         # compute_every could be a Symbol (:timestep). We process this that when we process
         # the list of diagnostics
         if !isa(compute_every, Symbol)
             output_every % compute_every == 0 || error(
-                "output_every should be multiple of compute_every for variable $(variable.long_name)",
+                "output_every should be multiple of compute_every for diagnostic $(descriptive_name)",
             )
         end
 
         isa_reduction = !isnothing(reduction_time_func)
 
         # If it is not a reduction, we compute only when we output
         if !isa_reduction && compute_every != output_every
-            @warn "output_every != compute_every for $(variable.long_name), changing compute_every to match"
+            @warn "output_every != compute_every for $(descriptive_name), changing compute_every to match"
             compute_every = output_every
         end
 
@@ -440,11 +452,17 @@ function ScheduledDiagnosticIterations(
         sd_time.compute_every == :timestep ? 1 : sd_time.compute_every / Δt
     output_every = sd_time.output_every / Δt
 
+    descriptive_name = get_descriptive_name(
+        sd_time.variable,
+        sd_time.output_every,
+        sd_time.reduction_time_func,
+    )
+
     isinteger(output_every) || error(
-        "output_every should be multiple of the timestep for variable $(sd_time.variable.long_name)",
+        "output_every should be multiple of the timestep for diagnostic $(descriptive_name)",
     )
     isinteger(compute_every) || error(
-        "compute_every should be multiple of the timestep for variable $(sd_time.variable.long_name)",
+        "compute_every should be multiple of the timestep for diagnostic $(descriptive_name)",
     )
 
     ScheduledDiagnosticIterations(;
@@ -500,10 +518,12 @@ ScheduledDiagnosticIterations(
     _Δt::T,
 ) where {T} = sd
 
+
 # We define all the known identities in reduction_identities.jl
 include("reduction_identities.jl")
 
-
+# Helper functions
+include("diagnostics_utils.jl")
 
 """
     get_callbacks_from_diagnostics(diagnostics, storage, counters)

src/diagnostics/Writers.jl

@@ -3,9 +3,8 @@
 # This file defines function-generating functions for output_writers for diagnostics. The
 # writers come with opinionated defaults.
 
-
 """
-    descriptive_name(sd_t::ScheduledDiagnosticTime)
+    get_descriptive_name(sd_t::ScheduledDiagnosticTime)
 
 
 Return a compact, unique-ish, identifier for the given `ScheduledDiagnosticTime` `sd_t`.
@@ -19,44 +18,15 @@ parameters such as whether there is a reduction in space or the compute
 frequency.
 
 """
-function descriptive_name(sd_t::ScheduledDiagnosticTime)
-    var = "$(sd_t.variable.short_name)"
-
-    isa_reduction = !isnothing(sd_t.reduction_time_func)
-
-    if isa_reduction
-        red = "$(sd_t.reduction_time_func)"
-
-        # Convert period from seconds to days, hours, minutes, seconds
-        period = ""
-
-        days, rem_seconds = divrem(sd_t.output_every, 24 * 60 * 60)
-        hours, rem_seconds = divrem(rem_seconds, 60 * 60)
-        minutes, seconds = divrem(rem_seconds, 60)
-
-        if days > 0
-            period *= "$(days)d_"
-        end
-        if hours > 0
-            period *= "$(hours)h_"
-        end
-        if minutes > 0
-            period *= "$(minutes)m_"
-        end
-        if seconds > 0
-            period *= "$(seconds)s_"
-        end
-
-        suffix = period * red
-    else
-        # Not a reduction
-        suffix = "inst"
-    end
-    return "$(var)_$(suffix)"
-end
+get_descriptive_name(sd_t::ScheduledDiagnosticTime) = get_descriptive_name(
+    sd_t.variable,
+    sd_t.output_every,
+    sd_t.reduction_time_func;
+    units_are_seconds = true,
+)
 
 """
-    descriptive_name(sd_i::ScheduledDiagnosticIterations, [Δt])
+    get_descriptive_name(sd_i::ScheduledDiagnosticIterations[, Δt])
 
 
 Return a compact, unique-ish, identifier for the given
@@ -71,20 +41,20 @@ other parameters such as whether there is a reduction in space or the compute
 frequency.
 
 """
-function descriptive_name(sd_i::ScheduledDiagnosticIterations,
-                          Δt = nothing)
-
-    if !isnothing(Δt)
-        # Convert iterations into time
-        return descriptive_name(ScheduledDiagnosticTime(sd_i, Δt))
-    else
-        var = "$(sd_i.variable.short_name)"
-        suffix =
-            isnothing(sd_i.reduction_time_func) ? "inst" :
-            "$(sd_i.output_every)it_(sd_i.reduction_time_func)"
-        return "$(var)_$(suffix)"
-    end
-end
+get_descriptive_name(sd_i::ScheduledDiagnosticIterations, Δt::Nothing) =
+    get_descriptive_name(
+        sd_t.variable,
+        sd_t.output_every,
+        sd_t.reduction_time_func;
+        units_are_seconds = false,
+    )
+get_descriptive_name(sd_i::ScheduledDiagnosticIterations, Δt::T) where {T} =
+    get_descriptive_name(
+        sd_i.variable,
+        sd_i.output_every * Δt,
+        sd_i.reduction_time_func;
+        units_are_seconds = true,
+    )
 
 
 """
@@ -118,12 +88,15 @@ function HDF5Writer()
     # and the integrator
     function write_to_hdf5(value, diagnostic, integrator)
         var = diagnostic.variable
-        time = lpad(integrator.t, 10, "0")
-        red = isnothing(diagnostic.reduction_time_func) ? "" : diagnostic.reduction_time_func
+        time = integrator.t
+
+        # diagnostic here is a ScheduledDiagnosticIteration. If we want to obtain a
+        # descriptive name (e.g., something with "daily"), we have to pass the timestep as
+        # well
 
         output_path = joinpath(
             integrator.p.simulation.output_dir,
-            "$(var.short_name)_$(red)_$(time).h5",
+            "$(get_descriptive_name(diagnostic, integrator.p.simulation.dt)).h5",
         )
 
         hdfwriter = InputOutput.HDF5Writer(output_path, integrator.p.comms_ctx)

src/diagnostics/diagnostics_utils.jl

@@ -0,0 +1,69 @@
+# diagnostic_utils.jl
+#
+# This file contains:
+# - get_descriptive_name: to condense ScheduledDiagnostic information into few characters.
+
+
+"""
+    get_descriptive_name(variable::DiagnosticVariable,
+                         output_every,
+                         reduction_time_func;
+                         units_are_seconds = true)
+
+
+Return a compact, unique-ish, identifier generated from the given information.
+
+`output_every` is interpreted as in seconds if `units_are_seconds` is `true`. Otherwise, it
+is interpreted as in units of number of iterations.
+
+This function is useful for filenames and error messages.
+
+ """
+
+function get_descriptive_name(
+    variable::DiagnosticVariable,
+    output_every,
+    reduction_time_func;
+    units_are_seconds = true,
+)
+    var = "$(variable.short_name)"
+    isa_reduction = !isnothing(reduction_time_func)
+
+    if units_are_seconds
+        if isa_reduction
+            red = "$(reduction_time_func)"
+
+            # Convert period from seconds to days, hours, minutes, seconds
+            period = ""
+
+            days, rem_seconds = divrem(output_every, 24 * 60 * 60)
+            hours, rem_seconds = divrem(rem_seconds, 60 * 60)
+            minutes, seconds = divrem(rem_seconds, 60)
+
+            if days > 0
+                period *= "$(days)d_"
+            end
+            if hours > 0
+                period *= "$(hours)h_"
+            end
+            if minutes > 0
+                period *= "$(minutes)m_"
+            end
+            if seconds > 0
+                period *= "$(seconds)s_"
+            end
+
+            suffix = period * red
+        else
+            # Not a reduction
+            suffix = "inst"
+        end
+    else
+        if isa_reduction
+            suffix = "$(output_every)it_(reduction_time_func)"
+        else
+            suffix = "inst"
+        end
+    end
+    return "$(var)_$(suffix)"
+end