From f44f5cc22c94ca5b6067eabe524853efebca8fe9 Mon Sep 17 00:00:00 2001 From: Allison Piper Date: Thu, 8 May 2025 10:02:54 -0400 Subject: [PATCH] Remove min-time/max-noise API. (#223) These are now owned by the stdrel stopping criterion, and should not be exposed directly in the benchmark/state/etc APIs. This will affect users that are calling `NVBENCH_BENCH(...).set_min_time(...)` or `NVBENCH_BENCH(...).set_max_noise(...)`. These can be updated to `NVBENCH_BENCH(...).set_criterion_param_float64(["min-time"|"max-noise"], ...)`. --- nvbench/benchmark_base.cuh | 34 ++++------------------------- nvbench/detail/measure_hot.cu | 4 +++- nvbench/detail/stdrel_criterion.cxx | 4 ++-- nvbench/json_printer.cu | 4 ---- nvbench/option_parser.cu | 5 ++++- nvbench/state.cuh | 34 ----------------------------- nvbench/stopping_criterion.cuh | 10 ++------- nvbench/stopping_criterion.cxx | 6 ----- testing/criterion_params.cu | 28 +----------------------- testing/device/noisy_bench.cu | 2 +- testing/option_parser.cu | 22 ------------------- testing/state_generator.cu | 6 ----- 12 files changed, 17 insertions(+), 142 deletions(-) diff --git a/nvbench/benchmark_base.cuh b/nvbench/benchmark_base.cuh index 829cccd..dce0afc 100644 --- a/nvbench/benchmark_base.cuh +++ b/nvbench/benchmark_base.cuh @@ -52,7 +52,9 @@ struct benchmark_base template explicit benchmark_base(TypeAxes type_axes) : m_axes(type_axes) - {} + { + this->set_stopping_criterion(nvbench::detail::default_stopping_criterion()); + } virtual ~benchmark_base(); @@ -190,34 +192,6 @@ struct benchmark_base } /// @} - /// Accumulate at least this many seconds of timing data per measurement. - /// Only applies to `stdrel` stopping criterion. @{ - [[nodiscard]] nvbench::float64_t get_min_time() const - { - return m_criterion_params.get_float64("min-time"); - } - benchmark_base &set_min_time(nvbench::float64_t min_time) - { - m_criterion_params.set_float64("min-time", min_time); - return *this; - } - /// @} - - /// Specify the maximum amount of noise if a measurement supports noise. - /// Noise is the relative standard deviation: - /// `noise = stdev / mean_time`. - /// Only applies to `stdrel` stopping criterion. @{ - [[nodiscard]] nvbench::float64_t get_max_noise() const - { - return m_criterion_params.get_float64("max-noise"); - } - benchmark_base &set_max_noise(nvbench::float64_t max_noise) - { - m_criterion_params.set_float64("max-noise", max_noise); - return *this; - } - /// @} - /// If a warmup run finishes in less than `skip_time`, the measurement will /// be skipped. /// Extremely fast kernels (< 5000 ns) often timeout before they can @@ -339,7 +313,7 @@ protected: nvbench::float32_t m_throttle_recovery_delay{0.05f}; // [seconds] nvbench::criterion_params m_criterion_params; - std::string m_stopping_criterion{"stdrel"}; + std::string m_stopping_criterion{}; private: // route these through virtuals so the templated subclass can inject type info diff --git a/nvbench/detail/measure_hot.cu b/nvbench/detail/measure_hot.cu index 202cda5..f44bb35 100644 --- a/nvbench/detail/measure_hot.cu +++ b/nvbench/detail/measure_hot.cu @@ -38,7 +38,9 @@ measure_hot_base::measure_hot_base(state &exec_state) : m_state{exec_state} , m_launch{exec_state.get_cuda_stream()} , m_min_samples{exec_state.get_min_samples()} - , m_min_time{exec_state.get_min_time()} + , m_min_time{exec_state.get_criterion_params().has_value("min-time") + ? exec_state.get_criterion_params().get_float64("min-time") + : 0.5} , m_skip_time{exec_state.get_skip_time()} , m_timeout{exec_state.get_timeout()} { diff --git a/nvbench/detail/stdrel_criterion.cxx b/nvbench/detail/stdrel_criterion.cxx index c0f7ef8..e4419bd 100644 --- a/nvbench/detail/stdrel_criterion.cxx +++ b/nvbench/detail/stdrel_criterion.cxx @@ -23,8 +23,8 @@ namespace nvbench::detail stdrel_criterion::stdrel_criterion() : stopping_criterion_base{"stdrel", - {{"max-noise", nvbench::detail::compat_max_noise()}, - {"min-time", nvbench::detail::compat_min_time()}}} + {{"max-noise", 0.005}, // 0.5% stdrel + {"min-time", 0.5}}} // 0.5 seconds {} void stdrel_criterion::do_initialize() diff --git a/nvbench/json_printer.cu b/nvbench/json_printer.cu index f3010a1..a62ee5c 100644 --- a/nvbench/json_printer.cu +++ b/nvbench/json_printer.cu @@ -311,8 +311,6 @@ void json_printer::do_print_benchmark_results(const benchmark_vector &benches) bench["index"] = bench_index; bench["min_samples"] = bench_ptr->get_min_samples(); - bench["min_time"] = bench_ptr->get_min_time(); - bench["max_noise"] = bench_ptr->get_max_noise(); bench["skip_time"] = bench_ptr->get_skip_time(); bench["timeout"] = bench_ptr->get_timeout(); @@ -370,8 +368,6 @@ void json_printer::do_print_benchmark_results(const benchmark_vector &benches) st["name"] = exec_state.get_axis_values_as_string(); st["min_samples"] = exec_state.get_min_samples(); - st["min_time"] = exec_state.get_min_time(); - st["max_noise"] = exec_state.get_max_noise(); st["skip_time"] = exec_state.get_skip_time(); st["timeout"] = exec_state.get_timeout(); diff --git a/nvbench/option_parser.cu b/nvbench/option_parser.cu index 58d3166..4bffa0f 100644 --- a/nvbench/option_parser.cu +++ b/nvbench/option_parser.cu @@ -1023,7 +1023,10 @@ try { // Any global params must either belong to the default criterion or follow a // `--stopping-criterion` arg: - nvbench::criterion_params params; + nvbench::criterion_params params = + criterion_manager::get() + .get_criterion(nvbench::detail::default_stopping_criterion()) + .get_params(); if (!params.has_value(name) && std::find(m_global_benchmark_args.cbegin(), m_global_benchmark_args.cend(), diff --git a/nvbench/state.cuh b/nvbench/state.cuh index dd4a675..9a8d915 100644 --- a/nvbench/state.cuh +++ b/nvbench/state.cuh @@ -170,40 +170,6 @@ struct state void set_disable_blocking_kernel(bool v) { m_disable_blocking_kernel = v; } /// @} - /// Accumulate at least this many seconds of timing data per measurement. - /// Only applies to `stdrel` stopping criterion. @{ - [[nodiscard]] nvbench::float64_t get_min_time() const - { - if (m_criterion_params.has_value("min-time")) - { - return m_criterion_params.get_float64("min-time"); - } - return 0.; - } - void set_min_time(nvbench::float64_t min_time) - { - m_criterion_params.set_float64("min-time", min_time); - } - /// @} - - /// Specify the maximum amount of noise if a measurement supports noise. - /// Noise is the relative standard deviation: - /// `noise = stdev / mean_time`. - /// Only applies to `stdrel` stopping criterion. @{ - [[nodiscard]] nvbench::float64_t get_max_noise() const - { - if (m_criterion_params.has_value("max-noise")) - { - return m_criterion_params.get_float64("max-noise"); - } - return 1.; - } - void set_max_noise(nvbench::float64_t max_noise) - { - m_criterion_params.set_float64("max-noise", max_noise); - } - /// @} - /// If a warmup run finishes in less than `skip_time`, the measurement will /// be skipped. /// Extremely fast kernels (< 5000 ns) often timeout before they can diff --git a/nvbench/stopping_criterion.cuh b/nvbench/stopping_criterion.cuh index 5daaaa5..77281f4 100644 --- a/nvbench/stopping_criterion.cuh +++ b/nvbench/stopping_criterion.cuh @@ -30,13 +30,7 @@ namespace nvbench namespace detail { - -constexpr nvbench::float64_t compat_min_time() { return 0.5; } // 0.5 seconds -constexpr nvbench::float64_t compat_max_noise() -{ - return 0.005; -} // 0.5% relative standard deviation - +inline std::string default_stopping_criterion() { return "stdrel"; } } // namespace detail /** @@ -47,7 +41,7 @@ class criterion_params nvbench::named_values m_named_values; public: - criterion_params(); + criterion_params() = default; criterion_params(std::initializer_list>); /** diff --git a/nvbench/stopping_criterion.cxx b/nvbench/stopping_criterion.cxx index f6a4ae5..7546650 100644 --- a/nvbench/stopping_criterion.cxx +++ b/nvbench/stopping_criterion.cxx @@ -22,12 +22,6 @@ namespace nvbench { -// Default constructor for compatibility with old code -criterion_params::criterion_params() - : criterion_params{{"max-noise", nvbench::detail::compat_max_noise()}, - {"min-time", nvbench::detail::compat_min_time()}} -{} - criterion_params::criterion_params( std::initializer_list> list) { diff --git a/testing/criterion_params.cu b/testing/criterion_params.cu index 92e2099..ae37a0f 100644 --- a/testing/criterion_params.cu +++ b/testing/criterion_params.cu @@ -21,27 +21,6 @@ #include "test_asserts.cuh" -void test_compat_parameters() -{ - nvbench::criterion_params params; - - ASSERT(params.has_value("max-noise")); - ASSERT(params.has_value("min-time")); - - ASSERT(params.get_float64("max-noise") == nvbench::detail::compat_max_noise()); - ASSERT(params.get_float64("min-time") == nvbench::detail::compat_min_time()); -} - -void test_compat_overwrite() -{ - nvbench::criterion_params params; - params.set_float64("max-noise", 40000.0); - params.set_float64("min-time", 42000.0); - - ASSERT(params.get_float64("max-noise") == 40000.0); - ASSERT(params.get_float64("min-time") == 42000.0); -} - void test_overwrite() { nvbench::criterion_params params; @@ -54,9 +33,4 @@ void test_overwrite() ASSERT(params.get_float64("custom") == 4.2); } -int main() -{ - test_compat_parameters(); - test_compat_overwrite(); - test_overwrite(); -} +int main() { test_overwrite(); } diff --git a/testing/device/noisy_bench.cu b/testing/device/noisy_bench.cu index 62aaab0..c76cc6d 100644 --- a/testing/device/noisy_bench.cu +++ b/testing/device/noisy_bench.cu @@ -140,4 +140,4 @@ NVBENCH_BENCH(noisy_bench) .add_float64_axis("Noise", {0.1, 5., 25.}) // % // disable this; we want to test that the benchmarking loop will still exit // when max_noise is never reached: - .set_max_noise(0.0000001); + .set_criterion_param_float64("max-noise", 0.0000001); diff --git a/testing/option_parser.cu b/testing/option_parser.cu index 36b8570..6104e5e 100644 --- a/testing/option_parser.cu +++ b/testing/option_parser.cu @@ -1155,26 +1155,6 @@ void test_min_samples() ASSERT(states[0].get_min_samples() == 12345); } -void test_min_time() -{ - nvbench::option_parser parser; - parser.parse({"--benchmark", "DummyBench", "--min-time", "12345e2"}); - const auto &states = parser_to_states(parser); - - ASSERT(states.size() == 1); - ASSERT(std::abs(states[0].get_min_time() - 12345e2) < 1.); -} - -void test_max_noise() -{ - nvbench::option_parser parser; - parser.parse({"--benchmark", "DummyBench", "--max-noise", "50.3"}); - const auto &states = parser_to_states(parser); - - ASSERT(states.size() == 1); - ASSERT(std::abs(states[0].get_max_noise() - 0.503) < 1.e-4); -} - void test_skip_time() { nvbench::option_parser parser; @@ -1486,8 +1466,6 @@ try test_axis_before_benchmark(); test_min_samples(); - test_min_time(); - test_max_noise(); test_skip_time(); test_timeout(); diff --git a/testing/state_generator.cu b/testing/state_generator.cu index 9042fef..14eceb5 100644 --- a/testing/state_generator.cu +++ b/testing/state_generator.cu @@ -763,8 +763,6 @@ void test_devices() void test_termination_criteria() { const nvbench::int64_t min_samples = 1000; - const nvbench::float64_t min_time = 2000; - const nvbench::float64_t max_noise = 3000; const nvbench::float64_t skip_time = 4000; const nvbench::float64_t timeout = 5000; @@ -774,8 +772,6 @@ void test_termination_criteria() dummy_bench bench; bench.set_devices(std::vector{}); bench.set_min_samples(min_samples); - bench.set_min_time(min_time); - bench.set_max_noise(max_noise); bench.set_skip_time(skip_time); bench.set_timeout(timeout); @@ -783,8 +779,6 @@ void test_termination_criteria() ASSERT(states.size() == 1); ASSERT(min_samples == states[0].get_min_samples()); - ASSERT(within_one(min_time, states[0].get_min_time())); - ASSERT(within_one(max_noise, states[0].get_max_noise())); ASSERT(within_one(skip_time, states[0].get_skip_time())); ASSERT(within_one(timeout, states[0].get_timeout())); }