From c91c4eb757651d81e5516f71cba15e81d40da429 Mon Sep 17 00:00:00 2001 From: Gabriele Bozzola Date: Sat, 17 Aug 2024 12:01:35 -0700 Subject: [PATCH] Add @with_error, remove @generate_error_functions `@generate_error_functions` was not a great macro: it required adding new names to a line every time a diagnostic is added, it was dealing with several variables at the same time, and could not be used outside of its scope of definition and/or called twice. `@with_error` accomplishes a the same goals as `@generate_error_functions` without the problems. The only difference is that now `compute` functions have to be decorated. --- docs/src/diagnostics/users_diagnostics.md | 26 +++++++++++++--- src/diagnostics/define_diagnostics.jl | 25 --------------- src/diagnostics/diagnostic.jl | 37 +++++++++++++++++++++++ src/diagnostics/land_compute_methods.jl | 24 ++++++++++----- test/diagnostics/diagnostics_tests.jl | 9 ++++++ 5 files changed, 84 insertions(+), 37 deletions(-) diff --git a/docs/src/diagnostics/users_diagnostics.md b/docs/src/diagnostics/users_diagnostics.md index b01aa58724..b474f4994a 100644 --- a/docs/src/diagnostics/users_diagnostics.md +++ b/docs/src/diagnostics/users_diagnostics.md @@ -66,16 +66,34 @@ Note that by default, `default_diagnostics` assign two optional kwargs: `output_ When defining a custom diagnostic, follow these steps: - Define how to compute your diagnostic variable from your model state and cache. - For example, let's say you want the bowen ratio (ratio between sensible heat and latent heat) in the Bucket model. - ``` - function compute_bowen_ratio!(out, Y, p, t, land_model::BucketModel) +For example, let's say you want the Bowen ratio (ratio between sensible heat and latent heat) in the Bucket model. +``` +function compute_bowen_ratio!(out, Y, p, t, land_model::BucketModel) if isnothing(out) return copy(p.bucket.turbulent_fluxes.shf / p.bucket.turbulent_fluxes.lhf) else out .= p.bucket.turbulent_fluxes.shf / p.bucket.turbulent_fluxes.lhf end end - ``` +``` +It is good practice to add error messages to inform other users that a diagnostic variable makes sense only with a +specific `land_model`. This can be accomplished by prepending the `@with_error` macro at the function declaration, +as in +``` +import ClimaLand.Diagnostics: @witherror + +@with_error function compute_bowen_ratio!(out, Y, p, t, land_model::BucketModel) + if isnothing(out) + return copy(p.bucket.turbulent_fluxes.shf / p.bucket.turbulent_fluxes.lhf) + else + out .= p.bucket.turbulent_fluxes.shf / p.bucket.turbulent_fluxes.lhf + end +end +``` +So, when someone tries outputting the Bowen ratio with a different model (e.g., `SnowModel`), `ClimaLand` will produce the following message: +``` +Cannot compute albedo with model = SnowModel +``` - Add that diagnostic variable to your list of variables ``` add_diagnostic_variable!( diff --git a/src/diagnostics/define_diagnostics.jl b/src/diagnostics/define_diagnostics.jl index 7663be92f3..872237d701 100644 --- a/src/diagnostics/define_diagnostics.jl +++ b/src/diagnostics/define_diagnostics.jl @@ -1,28 +1,3 @@ -# General helper functions for undefined diagnostics for a particular model -error_diagnostic_variable(variable, land_model::T) where {T} = - error("Cannot compute $variable with model = $T") - -# generate_error_functions is helper macro that generates the error message -# when the user tries calling something that is incompatible with the model -macro generate_error_functions(variable_names...) - functions = Expr[] - for variable in variable_names - function_name_sym = Symbol("compute_", variable, "!") - body = esc(quote - function $function_name_sym(_, _, _, _, land_model) - error_diagnostic_variable($variable, land_model) - end - end) - push!(functions, body) - end - return quote - $(functions...) - end -end - -# TODO: Automatically generate this list from the names of the diagnostics -@generate_error_functions "soil_net_radiation" "soil_latent_heat_flux" "soil_aerodynamic_resistance" "soil_sensible_heat_flux" "vapor_flux" "soil_temperature" "soil_water_liquid" "infiltration" "soilco2_diffusivity" "soilco2_source_microbe" "stomatal_conductance" "medlyn_term" "canopy_transpiration" "rainfall" "snowfall" "pressure" "wind_speed" "specific_humidity" "air_co2" "radiation_shortwave_down" "radiation_longwave_down" "photosynthesis_net_leaf" "photosynthesis_net_canopy" "respiration_leaf" "vcmax25" "photosynthetically_active_radiation" "photosynthetically_active_radiation_absorbed" "photosynthetically_active_radiation_reflected" "photosynthetically_active_radiation_transmitted" "near_infrared_radiation" "near_infrared_radiation_absorbed" "near_infrared_radiation_reflected" "near_infrared_radiation_transmitted" "radiation_shortwave_net" "radiation_longwave_net" "autotrophic_respiration" "soilco2" "heterotrophic_respiration" "soil_hydraulic_conductivity" "soil_water_potential" "soil_thermal_conductivity" "solar_zenith_angle" "moisture_stress_factor" "canopy_water_potential" "cross_section" "cross_section_roots" "area_index" "canopy_latent_heat_flux" "canopy_sensible_heat_flux" "canopy_aerodynamic_resistance" "canopy_temperature" "soil_ice" - """ define_diagnostics!(land_model) diff --git a/src/diagnostics/diagnostic.jl b/src/diagnostics/diagnostic.jl index fecc9b5fc9..c2f0ee1deb 100644 --- a/src/diagnostics/diagnostic.jl +++ b/src/diagnostics/diagnostic.jl @@ -88,6 +88,43 @@ function get_diagnostic_variable(short_name) return ALL_DIAGNOSTICS[short_name] end +# General helper functions for undefined diagnostics for a particular model +error_diagnostic_variable(variable, land_model::T) where {T} = + error("Cannot compute $variable with model = $(nameof(T))") + +# with_error is a helper macro that generates the error message +# when the user tries calling something that is incompatible with the model. +# It should be called when defining compute functions +macro with_error(compute_function_expr) + compute_function_expr.head == :function || + error("Cannot parse this function, head is not a :function") + + # Two firsts: + # 1st: extract the function signature + # 2nd: extract the name + function_name = first(first(compute_function_expr.args).args) + function_name isa Symbol || error("Cannot parse this function!") + + # qualified_name ensures that this macro can be used outside of this module while + # still defining compute functions in this module + qualified_name = GlobalRef(Diagnostics, function_name) + # Assuming the convention that functions are called "compute_variable!", + # otherwise the error might look a little less informative + variable_name = replace(string(function_name), "compute_" => "", "!" => "") + return esc( + quote + # Paste back the definition of the function + $compute_function_expr + # And add the error method, unless it was added by another macro call + if !(hasmethod($qualified_name, (Any, Any, Any, Any, Any))) + function $qualified_name(_, _, _, _, land_model) + error_diagnostic_variable($variable_name, land_model) + end + end + end, + ) +end + # Do you want to define more diagnostics? Add them here include("land_compute_methods.jl") diff --git a/src/diagnostics/land_compute_methods.jl b/src/diagnostics/land_compute_methods.jl index 9855112cf3..845ff63e59 100644 --- a/src/diagnostics/land_compute_methods.jl +++ b/src/diagnostics/land_compute_methods.jl @@ -21,15 +21,23 @@ end """ macro diagnostic_compute(name, model, compute) function_name = Symbol("compute_", name, "!") - return esc(quote - function $function_name(out, Y, p, t, land_model::$model) - if isnothing(out) - return copy($compute) - else - out .= $compute + return esc( + quote + @with_error function $function_name( + out, + Y, + p, + t, + land_model::$model, + ) + if isnothing(out) + return copy($compute) + else + out .= $compute + end end - end - end) + end, + ) end diff --git a/test/diagnostics/diagnostics_tests.jl b/test/diagnostics/diagnostics_tests.jl index 337db9320e..29ff4acdb6 100644 --- a/test/diagnostics/diagnostics_tests.jl +++ b/test/diagnostics/diagnostics_tests.jl @@ -3,6 +3,15 @@ using ClimaLand @test isdefined(ClimaLand.Diagnostics, :compute_albedo!) +@test !hasmethod( + ClimaLand.Diagnostics.compute_albedo!, + (Any, Any, Any, Any, Any), +) + +# Define some diagnostics for a DummyModel +struct DummyModel end +ClimaLand.Diagnostics.define_diagnostics!(DummyModel()) + # Just to trigger the error out = Y = p = t = land_model = nothing