ryanhaining / cppitertools

Implementation of python itertools and builtin iteration functions for C++17
https://twitter.com/cppitertools
BSD 2-Clause "Simplified" License
1.37k stars 115 forks source link

iter::combinations' behaviour when r = 0 is different from Python's itertools.combinations #101

Closed GaffaSnobb closed 10 months ago

GaffaSnobb commented 10 months ago

Consider these simple Python examples:

>>> list(itertools.combinations(iterable=[], r=1))
[]
>>> list(itertools.combinations(iterable=[], r=0))
[()]
>>> list(itertools.combinations(iterable=[1,2,3], r=0))
[()]

Note that when r is zero the return value is not an empty list, but rather a list containing an empty tuple, meaning that the returned list will iterate once:

>>> for e in list(itertools.combinations(iterable=[], r=0)):
...     print("Anyone here?")
...
Anyone here?

With iter::combinations however, I see that

std::vector<int> vec = {1,2,3};
auto combs = iter::combinations(vec, 0);

for (auto&& elem : combs)
{
    std::cout << "Anyone here?" << std::endl;
}

does not iterate. Maybe this is intended? My current use case of iter::combinations is to generate two different sets of combinations where r might be zero for either of them. I then iterate over every combination in the first set and for each of these combinations I iterate over every combination in the second set and concatenate them:

std::vector<std::vector<unsigned short>> basis_states;
for (auto&& proton_combination : proton_index_combinations)
{
    for (auto&& neutron_combination : neutron_index_combinations)
    {   
        std::vector<unsigned short> combined_index_combinations;
        for (unsigned short p : proton_combination)
        {
            combined_index_combinations.push_back(p);
        }
        for (unsigned short n : neutron_combination)
        {
            combined_index_combinations.push_back(n);
        }
        basis_states.push_back(combined_index_combinations);
    }
}

Because the object (combinator?) returned by iter::combinations does not iterate at all if r is zero, the vector basis_states will be empty if either of the combinations' r value is zero. With Python's itertools.combinations however, if the r value of one of the combinations is zero, then basis_states will simply be equal to the combination whose r value is non-zero.

As of now I have to do an if-else and separately handle the three possible cases (both non-zero, only first non-zero, only second non-zero) which of course works, but it could be prettier : )

Is this intentional or should it be fixed? If I had this problem in Python I'd simply append an empty tuple so that the list would iterate once, but I'm not quite sure how to do this with whatever object iter::combinations is giving me.

ryanhaining commented 10 months ago

Thanks for the detailed report and the repro steps. I agree this is wrong and should match python and have updated combinations and combinations_with_replacement to match. I'm sure there's a cleaner way than where I ended up, but it seems to work.

f I had this problem in Python I'd simply append an empty tuple so that the list would iterate once, but I'm not quite sure how to do this with whatever object iter::combinations is giving me.

This shouldn't matter at this point but iter::chain would probably be the way to try to append something extra.

GaffaSnobb commented 10 months ago

Thanks, mate! Appreciate the quick response!