Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a zip_axis / tuple_axis that iterates through multiple parameter axes in lockstep #68

Open
8 tasks
alliepiper opened this issue Jan 10, 2022 · 6 comments · May be fixed by #80
Open
8 tasks

Add a zip_axis / tuple_axis that iterates through multiple parameter axes in lockstep #68

alliepiper opened this issue Jan 10, 2022 · 6 comments · May be fixed by #80
Assignees
Labels
helps: rapids Helps or needed by RAPIDS. P2: nice to have Desired, but not necessary. type: enhancement New feature or request.

Comments

@alliepiper
Copy link
Collaborator

alliepiper commented Jan 10, 2022

Overview

RAPIDS needs a way to iterate through correlated axes in lockstep.

Example Usecase

Consider a benchmark that takes three parameters "X", "Y", and "Z", where X is an int, Y is a float, and Z is a string.

We want to run two instances of this benchmark:

  1. "X" is 402, "Y" is 0.6, and "Z" is "foo".
  2. "X" is 862, "Y" is 0.2, and "Z" is "bar".

Using regular axes here is troublesome, since those expand to a cartesian product by default. Defining these axes naively will produce 8 parametrizations.

Existing Solutions

Derived parameters

If Y and Z could be derived from X, we could just define X, compute Y and Z, then add summaries to the state for markdown output.

However, these parameters may not always be easily related, as in the example above.

Skipping

nvbench::state::skip provides a mechanism to skip some of the configurations in the cartesian product, and can be used to slice out just the configurations of interest.

However, this is tedious and fragile. You'd need to maintain some sort of validation logic that stays in sync with the axis values.

Lookup Tables

Each set of values could be put in a lookup function, and a single integer axis could be used to enumerate each desired set of parameters.

This is subpar. There is no way to override the actual values at runtime with -a options, only the index is modifiable. By default, the output will be cryptic, reporting only the test case index, not the actual values used. (This could be worked-around by hiding/adding summaries in the benchmark body).

Proposed Solution

Add a new zip_axis (tuple_axis?) type that appears as multiple distinct axes, but is defined by specifying discrete sets of inputs as tuples. This effectively adds an abstraction that simplifies the Lookup Table solution.

void my_bench(nvbench::state &state)
{
  const auto x = state.get_int64("X"); // 402, 862
  const auto y = state.get_int64("Y"); // 0.6, 0.2
  const auto z = state.get_int64("Z"); // foo, bar

  // ...
}

// (a)
auto my_values = nvbench::zip_axis_values<nvbench::int64_t, nvbench::float64_t, std::string>
{
  // Names for subaxes:
  {"X", "Y", "Z"},
  //  Tuples of grouped parameters
  {402, 0.6, "foo"},
  {862, 0.2, "bar"},
  // ...
};
NVBENCH_BENCH(my_bench)
  .add_zip_axis("ZippedValues", my_values);

// or

// (b)
NVBENCH_BENCH(my_bench)
  .add_zip_axis("ZippedValues",
                        "X", {402, 0.6},
                        "Y", {0.6, 0.2},
                        "Z", {"foo", "bar"});
  • The (a) form is nice because it lays out the grouped values together, making it easy to check that all subaxes are synced and have the same number of values.
  • The (b) form is convenient for small axes.

This should play nicely with other axes -- the cartesian product of all other axes + the zipped values should still be generated.

Open Questions

Command line interactions with -a

Proposed: If any subaxes are redefined, all subaxes must be redefined and have the same length, e.g.

my_bench -b 0 -a "X:[285,128,42]" -a "Y:[0.1,3.4,1.2]" -a "Z:[bing,bang,bong]"

If any axes are mismatched in length or unspecified, throw an error. Maybe we allow redefinition of a single subaxis only when the new definition is the same length as the hard-coded axis.

This could be punted on for the first version of this, since it'll be complicated no matter what approach we use, and it's not strictly necessary to meet the immediate need for zipped axes.

Markdown Output

Proposed: The zip axis should be transparent here -- ignore it and just treat the subaxes as regular axes:

|  X  |  Y  |   Z   | ... |
|-----|-----|-------|-----|
| 402 | 0.6 | "foo" | ... |
| 806 | 0.2 | "bar" | ... |

CSV Output

Proposed: Add columns with subaxes values, as well as a value that identifies the index in the zip, e.g.

ZippedValues, X, Y, Z, ...
0, 402, 0.6, "foo", ...
1, 806, 0.2, "bar", ....

JSON Output

This will be tricky and require some thought. We'll need to address both the per-benchmark axis definition as well as the per-state parameter encoding.

Implementation

This will likely be a new subclass of nvbench::axis_base that holds a vector of other nvbench::axis_bases.

Several areas will need to be updated, including but not limited to:

  • nvbench::axis_type enum
  • nvbench::option_parser for handling CLI
  • nvbench::markdown_printer, nvbench::csv_printer, nvbench::json_printer
  • nvbench::benchmark_base to add the add_zip_axis
  • nvbench::detail::state_generator and state_generator_iterator
  • nvbench::axis_metadata will need to learn how to handle these cleanly
  • Unit tests
  • Examples
@alliepiper alliepiper added type: enhancement New feature or request. P2: nice to have Desired, but not necessary. helps: rapids Helps or needed by RAPIDS. labels Jan 10, 2022
@jrhemstad
Copy link
Collaborator

I have a half-baked alternative idea.

The primary problem with the current value axii is that it will always generate the fully N-dimensional cross product for all N axii.

What if we could just tag individual axii as not participating in the cross product?

For example, if I wanted to just test at two points: {x, 1, a} and {y, 2, b}

If I were to specify these as 3 distinct axii like:

BM.add_string_axis("x", "y")
  .add_int64_axis(1, 2)
  .add_string_axis("a", "b");

Then that would result in the full cross product of 8 combinations. But what if I could somehow indicate that an axii shouldn't be treated as a dimension of the cross product? Or maybe we call this something different from an axis, something like a "list"?

The general intuition for how I think this would work would be to compute the cross product of all the axii, and for each tuple in the cross product, you append the corresponding element from the lists.

0 axis 3 lists

BM.add_string_list("x", "y")
  .add_int64_list(1, 2)
  .add_string_list("a", "b");

Would only generate two benchmarks with {x, 1, a} and {y, 2, b} as inputs.

1 axis, 2 lists

BM.add_string_axis("x", "y")
  .add_int64_list(1, 2)
  .add_string_list("a", "b");

Yields 2 benchmarks: {x, 1, a}, {y, 2, b}

(When there's only a single axis, it's the same as if it were a list)

2 axis, 1 list (uniform sizes)

BM.add_string_axis("x", "y")
  .add_int64_axis(1, 2)
  .add_string_list("a", "b");

Yields 4 benchmarks: {x, 1, a}, {x, 2, a}, {y, 1, b}, {y, 2, b}

2 axis, 1 list (different sizes)

Here's where I'm not quite sure what to do.

BM.add_string_axis("x", "y", "z")
  .add_int64_axis(1, 2, 3)
  .add_string_list("a", "b");

Following the general pattern I described above, we can first enumerate the cross product of the two axii:

{x, 1}
{x, 2}
{x, 3}
{y, 1}
{y, 2}
{y, 3}
{z, 1}
{z, 2}
{z, 3}

But what values from the list {a, b} do we associate with each tuple of the cross product? Do we require every list to be the same size as the largest axis?

There may be a more mathematical description/intuition for what I'm trying to describe from set theory, but I don't know set theory well enough to say for sure.

@alliepiper
Copy link
Collaborator Author

@jrhemstad Another consideration is how to represent a case with two zip axes. For instance:

NVBENCH_BENCH(my_bench)
  .add_zip_axis("ZipOne",
                "X", {1, 2},
                "Y", {"a", "b"})
  .add_zip_axis("ZipTwo",
                "U", {11, 12},
                "V", {"c", "d"});

would produce:

// {X, Y, U, V}
{ 1, a, 11, c }
{ 1, a, 12, d }
{ 2, b, 11, c }
{ 2, b, 12, d }

I don't think that can be handled with the list approach, since there's no way to associate sets of lists with each other.

@jrhemstad
Copy link
Collaborator

I don't think that can be handled with the list approach, since there's no way to associate sets of lists with each other.

Yep, you're completely correct. The zip axis is much better.

@robertmaynard
Copy link
Collaborator

Another option is to make the zipping/tieing of axes independent of creation of the axes. For example"

NVBENCH_BENCH(my+bench)
  .add_int64_axis("Int", {1, 2, 3})
  .add_float64_axis("Float", {11.0, 12.0, 13.0})
  .add_string_axis("String", {"One", "Two", "Three"})
  .tie_axii({"Int", "String"});

This has two major benifits in my mind. One it keeps the tieing and axis separate, making it easier to tie axis together on the command line. Second it avoids the type earsure implementation issues of zip axes.

@alliepiper
Copy link
Collaborator Author

The only drawback I see to that approach is that we'd lose the ability to specify "tied" axis values as tuples, e.g. this won't be possible anymore:

auto my_values = nvbench::zip_axis_values<nvbench::int64_t, nvbench::float64_t, std::string>
{
  // Names for subaxes:
  {"X", "Y", "Z"},
  //  Tuples of grouped parameters
  {402, 0.6, "foo"},
  {862, 0.2, "bar"},
  // ...
};

However, I agree that this would be much simpler to implement without a major loss of functionality, and would greatly simplify a lot of the output format and CLI issues. The simpler implementation would be worthwhile IMO 👍

@alliepiper
Copy link
Collaborator Author

One minor note -- we've been using axes for the plural of axis, so s/axii/axes/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helps: rapids Helps or needed by RAPIDS. P2: nice to have Desired, but not necessary. type: enhancement New feature or request.
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants