Address PR comments: rename hcat_timed, add tests, adjust threshold

src/PowerAnalytics.jl

@@ -18,7 +18,7 @@ export is_col_meta, set_col_meta, set_col_meta!, get_time_df, get_time_vec, get_
     get_data_df, get_data_vec, get_data_mat, get_description, get_component_agg_fn,
     get_time_agg_fn, with_component_agg_fn, with_time_agg_fn, metric_selector_to_string,
     get_agg_meta, set_agg_meta!, rebuild_metric
-export compute, compute_all, hcat_timed, aggregate_time, compose_metrics
+export compute, compute_all, hcat_timed_dfs, aggregate_time, compose_metrics
 export create_problem_results_dict
 export parse_generator_mapping_file, parse_injector_categories, parse_generator_categories
 export mean, weighted_mean, unweighted_sum

src/builtin_component_selectors.jl

@@ -94,7 +94,7 @@ function parse_generator_mapping_file(
         # A subselector will be nothing if it doesn't fit under root_type
         subselectors = filter(!isnothing, subselectors)
         # Omit the category entirely if root_type causes elimination of all subselectors
-        length(in_data[top_level]) > 0 && length(subselectors) == 0 && continue
+        (length(in_data[top_level]) > 0 && isempty(subselectors)) && continue
         mappings[top_level] = make_selector(subselectors...; name = top_level)
     end
     return mappings, get(in_data, FUEL_TYPES_META_KEY, nothing)

src/builtin_metrics.jl

@@ -1,4 +1,3 @@
-# TODO test
 "Convenience function to convert an EntryType to a function and make a ComponentTimedMetric from it"
 make_component_metric_from_entry(
     name::String,
@@ -8,7 +7,6 @@ make_component_metric_from_entry(
         eval_fn = (res::IS.Results, comp::Component; kwargs...) ->
             read_component_result(res, key, comp; kwargs...))
 
-# TODO test
 "Convenience function to convert a SystemEntryType to a function and make a SystemTimedMetric from it"
 make_system_metric_from_entry(
     name::String,
@@ -222,7 +220,7 @@ const calc_integration = ComponentTimedMetric(;
         (res::IS.Results, comp::Component; kwargs...
         ) -> let
             result = compute(calc_active_power, res, comp; kwargs...)
-            # TODO does not check date alignment, maybe use hcat_timed
+            # TODO does not check date alignment, maybe use hcat_timed_dfs
             denom = (.+)(
                 (_integration_denoms(res; kwargs...) .|> get_data_vec .|> collect)...,
             )
@@ -235,7 +233,7 @@ const calc_integration = ComponentTimedMetric(;
     # We use a custom eval_zero to put the weight in there even when there are no components
     eval_zero = (res::IS.Results; kwargs...) -> let
         denoms = _integration_denoms(res; kwargs...)
-        # TODO does not check date alignment, maybe use hcat_timed
+        # TODO does not check date alignment, maybe use hcat_timed_dfs
         time_col = get_time_vec(first(denoms))
         data_col = repeat([0.0], length(time_col))
         result = DataFrame(DATETIME_COL => time_col, "" => data_col)
@@ -345,7 +343,7 @@ make_calc_is_slack_up(threshold::Real) = SystemTimedMetric(;
 )
 
 # TODO is this the appropriate place to put a default threshold (and is it the appropriate default)?
-const DEFAULT_SLACK_UP_THRESHOLD = 1e-3
+const DEFAULT_SLACK_UP_THRESHOLD = 1e-4
 "Calculate whether the given time period has system balance slack up of magnitude greater than $DEFAULT_SLACK_UP_THRESHOLD"
 const calc_is_slack_up = make_calc_is_slack_up(DEFAULT_SLACK_UP_THRESHOLD)
 

src/metrics.jl

@@ -299,7 +299,7 @@ function compute(metric::ComponentTimedMetric, results::IS.Results,
     selector::ComponentSelector; kwargs...)
     subents = get_groups(selector, results)
     subcomputations = [_compute_one(metric, results, sub; kwargs...) for sub in subents]
-    return hcat_timed(subcomputations...)
+    return hcat_timed_dfs(subcomputations...)
 end
 
 # COMPUTE_ALL()
@@ -356,7 +356,7 @@ compute_all(results::IS.Results,
     selectors::Union{Nothing, Component, ComponentSelector, Vector} = nothing,
     col_names::Union{Nothing, Vector{<:Union{Nothing, AbstractString}}} = nothing;
     kwargs...,
-) = hcat_timed(_common_compute_all(results, metrics, selectors, col_names; kwargs)...)
+) = hcat_timed_dfs(_common_compute_all(results, metrics, selectors, col_names; kwargs)...)
 
 """
 For each (metric, col_name) tuple in `zip(metrics, col_names)`, call [`compute`](@ref) and

src/output_utils.jl

@@ -119,7 +119,7 @@ end
 If the time axes match across all the `DataFrames`, horizontally concatenate them and remove
 the duplicate time axes. If not, throw an error
 """
-function hcat_timed(vals::DataFrame...)  # TODO incorporate allow_missing
+function hcat_timed_dfs(vals::DataFrame...)  # TODO incorporate allow_missing
     time_col = _extract_common_time(vals...; ex_fn = get_time_df)
     broadcasted_vals = [get_data_df(sub) for sub in _broadcast_time.(vals, Ref(time_col))]
     return hcat(time_col, broadcasted_vals...)

test/test_builtin_metrics.jl

@@ -5,6 +5,28 @@
 (results_uc, results_ed) = run_test_sim(TEST_RESULT_DIR, TEST_SIM_NAME)
 const ResultType = AbstractDataFrame
 
+@testset "Test make_component_metric_from_entry" begin
+    entry = PSI.ActivePowerVariable
+    my_calc_active_power = PA.make_component_metric_from_entry("MyActivePower", entry)
+    @test get_name(my_calc_active_power) == "MyActivePower"
+    comp = get_component(ThermalStandard, get_system(results_uc), "Solitude")
+    my_result = my_calc_active_power(make_selector(comp), results_uc)
+    existing_result = PA.read_component_result(results_uc, entry, comp)
+    @test get_time_vec(my_result) == get_time_vec(existing_result)
+    @test get_data_vec(my_result) == get_data_vec(existing_result)
+end
+
+@testset "Test make_system_metric_from_entry" begin
+    entry = PSI.SystemBalanceSlackUp
+    my_calc_system_slack_up = PA.make_system_metric_from_entry("MySystemSlackUp", entry)
+    @test get_name(my_calc_system_slack_up) == "MySystemSlackUp"
+    my_result = my_calc_system_slack_up(results_ed)
+    existing_result = PA.read_system_result(results_ed, entry)
+    @test get_time_vec(my_result) == get_time_vec(existing_result)
+    @test get_data_vec(my_result) == get_data_vec(existing_result)
+end
+
+# TEST EACH BUILT-IN METRIC INDIVIDUALLY
 # Future implementers of built-in metrics must add tests as shown below or this will trigger
 function test_metric(::Val{metric_name}) where {metric_name}
     throw("Could not find test for $metric_name")

test/test_metrics.jl

@@ -262,7 +262,10 @@ end
     @test_throws ArgumentError get_data_vec(my_df2)
     @test get_data_mat(my_df2) == hcat(copy(my_data1), copy(my_data2))
 
-    @test hcat_timed(my_df1, DataFrames.rename(my_df1, "MyComponent" => "YourComponent")) ==
+    @test hcat_timed_dfs(
+        my_df1,
+        DataFrames.rename(my_df1, "MyComponent" => "YourComponent"),
+    ) ==
           DataFrame(
         DATETIME_COL => my_dates,
         "MyComponent" => my_data1,