Batched min-time is modeled as a stdrel criterion parameter, causing unintuitive CLI behavior with other stopping criteria

[oleksandr-pavlyk]

--min-time options is currently treated as a stopping-criterion parameter owned by stdrel.

This creates surprising CLI behavior when users select another stopping criterion such as sample-count, entropy, or a custom criterion. For example:

./build/bin/nvbench.example.cpp20.throughput \
    --stopping-criterion sample-count \
    --target-samples 3 \
    --min-time 1e-5

fails with:

NVBench encountered an error:

/home/opavlyk/repos/nvbench/nvbench/option_parser.cu:1107: Error handling option `--min-time 1e-5`:
/home/opavlyk/repos/nvbench/nvbench/option_parser.cu:1073: Unrecognized stopping criterion \
      parameter: `min-time` for `sample-count`.

The same happens with entropy:

./build/bin/nvbench.example.cpp20.throughput \
   --stopping-criterion entropy \
   --min-time 1e-5

Confusingly, this ordering works:

./build/bin/nvbench.example.cpp20.throughput \
    --min-time 1e-5 \
    --stopping-criterion entropy

but --min-time does not actually apply to "entropy". It is accepted while the default "stdrel" criterion is active, then --stopping-criterion entropy resets the benchmark criterion parameters.

Conceptually, --min-time does not seem like it should be a "stdrel"-specific stopping-criterion parameter.

It is used by hot/batched measurement logic, where it behaves more like a benchmark or measurement option.

Suggested improvements:

1. Keep rejecting invalid options, but improve the error message. For example:

--min-time is only valid for the 'stdrel' stopping criterion in the current CLI model.
  Current criterion 'entropy' accepts: --max-angle, --min-r2.

2. Move min-time out of 'stdrel; criterion params and model it as a benchmark/measurement option, or introduce a more explicit hot/batch option such as --batch-min-time.

3. Once min-time is no longer criterion-owned, simplify the example CTest logic and remove or reduce the need for no_min_time_examples.

[oleksandr-pavlyk]

Background

Presently, --min-time is parsed as and is stored as default "stdrel" stopping-criterion's parameter. "stdrel" requires specified total kernel execution time (as measured by device timer) to be greater than "min-time" value before statistics-based stopping criterion is applied.

The issue is that hot-measuring loop (see measure_hot class constructor in 'measure_hot.cu') uses that parameter from stdrel criterion (if active, or defaults to 0.5 seconds otherwise).

measure_hot_base::measure_hot_base(state &exec_state)
    : m_state{exec_state}
    ...
    , m_min_time{exec_state.get_criterion_params().has_value("min-time")
                   ? exec_state.get_criterion_params().get_float64("min-time")
                   : 0.5}
    ...
{

This makes min-time a measurement parameter, rather than stdrel criterion parameter.

Perhaps every measuring loop, i.e., measure_cold, measure_hot, and measure_cpu_only, must accept min_time parameter the same way they accept min_samples parameter (i.e. benchmark_base and state store m_min_time and m_min_samples).

Proposed solution

It is right to store --min-time the same way the --min-samples is stored, i.e. in benchmark_base and state objects).

Even though the entity that encodes measurement loop logic owns m_min_time and m_min_samples it is not the right behavior for every stopping criterion to require that total_time >= m_min_time and total_samples >= m_min_sample before criterion's logic is allowed to decide whether to finish further data collection.

The 'sample-targets' stopping criterion wants to ensure that number of samples collected is exactly the requested number, even if that number is smaller than the --min-samples implies.

One way to accomplish this is to extend stopping_criterion_base with a method that decides whether is_finished() call should be short-circuited. Let's call this method virtual bool is_eligible_to_stop(stopping_context ctx) const, where

struct stopping_context
{
    nvbench::int64_t total_samples;
    nvbench::float64_t total_time;
    nvbench::int64_t min_samples;
    nvbench::float64_t min_time;
};

The default implementation of the method would be

virtual bool is_eligible_to_stop(stopping_context ctx) const {
   return (ctx.total_samples >= ctx.min_samples) && (ctx.total_time >= ctx.min_time);
}

and the m_stopping_criterion.is_finished() becomes m_stopping_criterion.is_eligible_to_stop(ctx) && m_stopping_criterion.is_finished() in cold and cpu-only measurement loops.

[oleksandr-pavlyk]

State of implementation

min-samples

Parameter --min-samples is stored in benchmark_base::m_min_samples, passed to state, and so is accessible to all measurement loops, i.e. cold measurements, batched measurements, and cpu-only measurements.

Each of these measurement loops ensures that the actual number of executions of user-supplied launchable is greater than m_min_samples.

min-time in stdrel-stopping criterion

'stdrel' stopping criterion (based on coefficient of variation $C = \frac{\sigma}{\mu}$, often referred to as relative standard deviation in NVBench) has parameter "min-time" which defaults to 0.5.

Stopping criteria are entities used to decide when the cold and the cpu-only measurement loops are done collection of time-measurements of execution of the launchable by checking return value of stopping_criterion_base::is_finished() method. In these two measurement loops, user-chosen stopping criterion is applied only after m_min_samples data-points have been accumulated.

'stdrel' criterion further short-circuits its is_finished() method unless total measured launchable execution time becomes greater than requested min-time:

  if (total_measured_time <= m_params.get_float64("min-time"))
  {
    return false;
  }

CLI parameter --min-time is treated by option parser as stopping-criterion parameter, and hence requires 'stdrel' stopping criterion to be active.

min-time in batched measurements

Use of measure_hot_base::m_min_time is encoded in measure_hot_base::run_trials.

m_min_time and m_min_samples both dictate when the batch measurement loop terminates:

      if (m_total_cuda_time > m_min_time && // min time okay
          m_total_samples >= m_min_samples) // min samples okay
      {
        break; // Stop iterating
      }

In addition, m_min_time is also used to determine batch size.

      const auto remaining_time = m_min_time - m_total_cuda_time;
      const auto time_per_sample =
        m_total_cuda_time / static_cast<nvbench::float64_t>(m_total_samples);
      batch_size = static_cast<nvbench::int64_t>(remaining_time / time_per_sample);

Presently, measure_hot_base::m_min_time is set based on stopping-criterion associated with benchmark, if it has "min-time" parameter, otherwise the default hard-coded value of 0.5 is used.

Therefore there is no way to control this parameter after "entropy" stopping criterion (which does not have "min-time" parameter) is specified. The work-around is to set --min-time first, when stdrel is still considered active, before changing stopping-criterion away from 'stdrel' default.

User-facing proposals

Proposal 1

Promote --min-time to measurement options and treat it like --min-samples. The --min-time option becomes valid irrespective of which stopping criterion option parser considers active.

Every measurement loop ensures total_num_samples >= m_min_samples && total_time >= m_min_time before it can terminate. For cold and cpu-only measurement loops that would mean that stopping_criterion_base::is_finished() is only called after that condition is met.

Advantages:

• --min-time parameter becomes easy to explain, and CLI option --min-time becomes universally valid;
• default value can be set to 0.0 (disabled), suitable for "entropy" and "sample-count" stopping criteria.
  • batched measurement's batch size computation will need to be tweaked, so that if min-time-based estimate for batch-size is too low, we'd use max(m_min_samples, 1) launches instead of presently used value 1.
  • stdrel_criterion::is_finished() may use low positive hard-coded threshold value and additionally require total_time >= threshold

Disadvantage:

• The choice of default value 0.0 changes behavior of batched measurements loop, and default stdrel stopping criterion, while choice of default value 0.5 changes "entropy" stopping criterion behavior.
  • Perhaps a way to resolve this difficulty is to make m_min_time have type std::optional<nvbench::float64_t> with default value of std::nullopt. The nullopt would be treated as presently used hard-coded default in batched loop, and in other loops replaced with default value provided by a virtual method of stopping criterion.

Proposal 2

Keep 'min-time' as stdrel stopping criterion parameter only, and introduce a dedicated option to control min-time for batched measurements loop, say --batched-min-time with the default value of 0.5.

Advantages:

• Backwards compatible

Disadvantage:

• Hard to explain existence of and distinction between --min-time and --batched-min-time CLI options to end-users.

Proposal 3

Like suggested in the previous comment, and like with proposal 1, --min-time is promoted to measurement options and becomes a valid CLI option irrespective of the active stopping criterion.

Measurement loops that use stopping criteria, i.e., cold measurements and cpu-only measurements loops, delegate how to treat --min-time and --min-samples values to stopping criteria themselves via bool stopping_criterion_base::is_eligible_to_stop( stopping_context context).

Advantages:

• --min-time becomes a global option, allowing a user to control minimal duration of batched measurement loop regardless of stopping criterion chosen
• Individual stopping criteria can choose whether to honor --min-samples/--min-time as appropriate. --stopping-criterion sample-count wants the count requested via its criterion parameter to always be honored, --stopping-criterion entropy may choose to ignore min-time requested for backward compatibility

Disadvantages:

• It becomes harder to explain --min-time/--min-samples as their effect become dependent on the stopping criterion chosen in cold and cpu-only measurement loops. In batched measurements loop, both constraints must be satisfied. However, since batched measurements loop is always executed after cold measurements loop, it is not clear whether the same constraint values should be used for both loops.

[oleksandr-pavlyk]

From a stand-point of usability.

1. I recognize need for an alternative to --min-samples expressed as duration because benchmarks may contain kernels with vastly varying typical runtimes. So --min-samples would need to be higher for short-running kernels and lower for heavier kernels. Using required execution time is more meaningful in this case.
   
   From this vantage point, exit condition should be using || rather than &&, i.e., num_samples >= m_min_samples || total_time >= m_min_time.
   
   I see several reasons to require min-samples/min-time. One is to ensure that benchmark reaches desired system state before stopping criterion stops the run. Another is to override logic of stopping criterion to prevent it from stopping prematurely.

2. In batch measurements, the core user-configurable knob is batch size to use. The loop termination criterion logic is hard coded.

I would say that premature stop guarding should really be stopping criterion parameters because what we are guarding against is stopping-criterion specific.

The warm-up state settling is already handled by --cold-warmup-runs. It is expressed in number of warm-up launches to perform. There is no way to express it as warm-up for at least this long. There is an option to prevent warmup from taking too long, i.e., --cold-max-warmup-walltime.

For batched loop, we need a knob to control batch size, and we need to control when batched loop terminates (because it does not use stopping criterion).

These considerations lead to the following minimal practical proposal:

• --min-samples: remains the existing common sample-count lower bound.
• --min-time: remains stdrel-specific
• --batch-target-time: new batched measurement option, valid regardless of stopping criterion used by cold/cpu_only loop.

I am going to open an alternative to #424 that implements this change.