Conversation
Krzmbrzl
added
bug
Krzmbrzl
force-pushed
the
revamp-count-cycles
branch
3 times, most recently
from
c16af57 to
3ef7dde
Compare
Krzmbrzl
force-pushed
the
revamp-count-cycles
branch
from
3ef7dde to
4f035e4
Compare
|
|
||
| idx1 = std::distance(v1.begin(), it1); | ||
| SEQUANT_ASSERT(idx1 >= 0); | ||
| it0 = std::find(begin(v0), end(v0), *val_it); |
There was a problem hiding this comment.
isn't it0 always equal to val_it? There are no duplicates in v0 (or v1)...
There was a problem hiding this comment.
I thought so, too. But actually that's not true. Consider this example:
┎ it0
| ┎ val_it
i1 i2 i3
i3 i1 i2
┖ it1
The thing is that we have to perform a positional lookup (look for the thing that is in the same column as it1 but in v0, rather than looking for a specific element (that one of the other iterators would point to).
When using one of the two iterators instead of val_it, we get an endless loop (which is evident from the above sketch).
| std::remove_reference_t<Seq0> v(std::forward<Seq0>(v0)); | ||
| using T = std::decay_t<decltype(v[0])>; | ||
| SEQUANT_ASSERT(ranges::is_permutation(v, v1)); | ||
| std::size_t count_cycles(Seq0&& v0, Seq1&& v1) { |
There was a problem hiding this comment.
can probably take both via const lvalue ref?
There was a problem hiding this comment.
Unfortunately no - ranges can't be iterated (in general) when they are const. Hence, the only way to write a function that works for generic ranges is by taking them as non-const. Personally, I consider this a major flaw in the standard, but that's what it is... ¯_(ツ)_/¯
2 participants
No description provided.