From 63486aca496c2770d5f1b543f8eed69215d895b0 Mon Sep 17 00:00:00 2001 From: GabrielKS <23368820+GabrielKS@users.noreply.github.com> Date: Fri, 31 Jan 2025 11:40:05 -0700 Subject: [PATCH] Address PR comments: rename `hcat_timed`, add tests, adjust threshold --- src/PowerAnalytics.jl | 2 +- src/builtin_component_selectors.jl | 2 +- src/builtin_metrics.jl | 8 +++----- src/metrics.jl | 4 ++-- src/output_utils.jl | 2 +- test/test_builtin_metrics.jl | 22 ++++++++++++++++++++++ test/test_metrics.jl | 5 ++++- 7 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/PowerAnalytics.jl b/src/PowerAnalytics.jl index da37237..650bdf4 100644 --- a/src/PowerAnalytics.jl +++ b/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 diff --git a/src/builtin_component_selectors.jl b/src/builtin_component_selectors.jl index 9567e69..18501da 100644 --- a/src/builtin_component_selectors.jl +++ b/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) diff --git a/src/builtin_metrics.jl b/src/builtin_metrics.jl index 3523209..1c45b38 100644 --- a/src/builtin_metrics.jl +++ b/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) diff --git a/src/metrics.jl b/src/metrics.jl index dde800c..5e81158 100644 --- a/src/metrics.jl +++ b/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 diff --git a/src/output_utils.jl b/src/output_utils.jl index b5a8d65..1e8cacb 100644 --- a/src/output_utils.jl +++ b/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...) diff --git a/test/test_builtin_metrics.jl b/test/test_builtin_metrics.jl index cd29ed4..bf58185 100644 --- a/test/test_builtin_metrics.jl +++ b/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") diff --git a/test/test_metrics.jl b/test/test_metrics.jl index 40a86aa..2acdbc4 100644 --- a/test/test_metrics.jl +++ b/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,