Skip to content

MultiProductArray/iproduct_arr #286

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tobz1000
Copy link
Contributor

@tobz1000 tobz1000 commented Jun 1, 2018

Fixes #264.

I've added a new macro, iproduct_arr, which can be called in the same way as iproduct (but only when all iterators are the same type) or using the [iter; count] format, as requested in the issue.

The macro uses a new underlying type, MultiProductArray, which is like a MultiProduct but yields arrays instead of vecs. The short-form macro uses a repeat_n rather than macro-based repetition, so there shouldn't be any tricky ownership/multiple-mutation issues.

I did consider using something similar to the combination of ConsTuples+Product which iproduct uses, but found that utilising MultiProduct as a base was a better fit. The tradeoffs of this decision are as follows:

  • A user can create their own MultiProductArray without using the macro, and in this case use a meta-iterator as the source, as long as the number of produced sub-iterators is known at compile-time. (e.g. (0..3).map(|i| 0..i))
  • The benchmark tests show that it is about 1.25x faster in the for-loop, but about 6.5x slower in the .fold() (and still many times faster than the vec-based MultiProduct). I haven't benched with types which allocate heap memory.

Creating a MultiProductArray manually requires the user to specify an array type parameter with length matching the number of sub-iterators. It uses a trait, implemented only on array types, to assert that this length is correct upon construction. The trait is "public-private", so that it cannot be implemented by users.

I've updated the documentation for iproduct and multi_cartesian_product to compare them with iproduct_arr.

While adding quickcheck tests, I refactored the main tests for Product, MultiProduct and MultiProductArray to use the same underlying function. MultiProduct's test should be functionally identical to before, whilst Product now has slightly different input variables, and additionally tests .last().

During development, I ended up doing a fairly significant refactor of MultiProduct. The original reason for this was a potential performance improvement; however I didn't realise that the improvement is not possible on rustc 1.12 (without further impl type restrictions). As a result, the current implementation would probably work without this refactor. So there are probably more changes for you to review than strictly necessary... but it does also introduce a ~5% speed increase for MultiProduct :)

Also, I added a match-arm to both iproduct and iproduct_arr to allow trailing commas.

As always, let me know if there are any API/syntax changes that you think should be made, plus any other issues you see with the aforementioned changes.

Adds the `iproduct_arr` macro and its underlying type.

Functions like the `iproduct` macro, but yields arrays rather than tuples, and allows the shorthand form `iproduct_arr![iter; count]` for repeated iterators.
@tobz1000 tobz1000 force-pushed the iproduct-arr-squash branch from 920164a to c7c8cf7 Compare June 1, 2018 23:41
@jswrenn jswrenn self-assigned this Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: n-dimensional product over the same iterator
2 participants