From 40d2941eff899cb7199b6b430b3c21ece5fea780 Mon Sep 17 00:00:00 2001 From: Gabriele Bozzola Date: Wed, 6 Sep 2023 08:36:43 -0700 Subject: [PATCH] Use descriptive_name for filenames and errors --- src/diagnostics/Diagnostics.jl | 38 ++++++++++--- src/diagnostics/Writers.jl | 83 ++++++++++------------------ src/diagnostics/diagnostics_utils.jl | 69 +++++++++++++++++++++++ 3 files changed, 126 insertions(+), 64 deletions(-) create mode 100644 src/diagnostics/diagnostics_utils.jl diff --git a/src/diagnostics/Diagnostics.jl b/src/diagnostics/Diagnostics.jl index 43596c50955..85e96963356 100644 --- a/src/diagnostics/Diagnostics.jl +++ b/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,12 +388,18 @@ 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 @@ -395,7 +407,7 @@ struct ScheduledDiagnosticTime{T1, T2, OW, F1, F2, PO} # 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) diff --git a/src/diagnostics/Writers.jl b/src/diagnostics/Writers.jl index 006a9b34456..6cd13d77079 100644 --- a/src/diagnostics/Writers.jl +++ b/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) diff --git a/src/diagnostics/diagnostics_utils.jl b/src/diagnostics/diagnostics_utils.jl new file mode 100644 index 00000000000..ae1ae4d7492 --- /dev/null +++ b/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