Pairwise Alignment with ranges and with_alignment doesn't compile
#1598
Comments
|
Does the same error occur with |
Good Idea! |
|
I thought as much. |
|
My changes have been incorporated into |
|
(I encountered this problem when the ranges library did the change with the safe_ranges #1471, but haven't thought to deeply about it.) The problem in this case is manifold. 1.) The stack trace should be nicer, but unfortunately gcc<=9 does not evaluate concepts on friend functions seqan3/include/seqan3/range/decorator/gap_decorator.hpp Lines 291 to 298 in 2763c90 2.) We require that seqan3/include/seqan3/range/decorator/gap_decorator.hpp Lines 134 to 141 in 2763c90 3.) We re-assign in the The main problem is that our Thus, we do an assignment of The interesting thing is why our stuff builds at all is due to the type erasure. The operation seqan3/include/seqan3/alignment/matrix/alignment_trace_algorithms.hpp Lines 124 to 125 in 2763c90 The fix should be simple, |
|
@marehr Yes, your assessment is correct in all points! Regarding the solution I would suggest a slightly different path:
This means that if the underlying view type of the decorator is e.g. Secondly, I would define [1] It does mean that you won't be able to assign a slice for type combinations without type erasure, but in those cases it would be weird anyway, or not? |
…:pair On step to fix issue seqan#1598. The sequence builder uses internally a pair to represent a sequence alignment that will be implicitly converted to a std::tuple in the final returned result.
But why is that? When my sequence type is not type erasable the code wouldn't work, would it? |
|
Sure, Right now the gap_decorator requires random_access+sized so all input can be erased. If we loosen that restriction, I still don't see many use-cases for assigning the slice-type, after all, if your range is not sized, how would you create the slice anyway? Yes, there are non-random-access sized ranges, but it looks like a weird design to me. |
|
I am not sure I understand everything. using dec_t = decltype(gap_decorator{std::declval<std::vector<char>&>() | views::type_reduce});
std::vector<char> my_vec{...};
dec_t instance{my_vec | views::slice}So here the vector is type reduced to a span and slice over the vector would also deduce to a span. Accordingly, gap decorator can be assigned from another span and there is no need to define the type of the gap decorator from a sliced range. But when my vector is in fact a reversed vector this doesn't hold anymore, does it? using dec_t = decltype(gap_decorator{std::declval<std::vector<char>&>() | std::views::reverse});
std::vector<char> my_vec{...};
dec_t instance{my_vec | std::views::reverse | views::slice}Reverse does not type erase on a vector and returns a random access, sized view, which is also not a borrowed range so no erasure here, right? Then assigning a sliced reverse view to a reverse view would not work. Or did something change here in the standard? Many thanks for clarifying this. |
|
I am suggesting that the
In both cases the type with and without slicing is the same. See https://github.com/seqan/seqan3/blob/master/include/seqan3/range/views/type_reduce.hpp#L75 |
|
That is not true: So adding the type_reduce does not help at all for a very regular use case. The reasons is that a view is always forwarded as is in the type_reduce closure object: |
Fixes #1598: pairwise alignment does not work for some ranges
Platform
Linux schenker 5.5.4-arch1-1 #1 SMP PREEMPT Sat, 15 Feb 2020 00:36:29 +0000 x86_64 GNU/Linuxgcc (Arch Linux 9.2.1+20200130-2) 9.2.1 20200130Description
I followed the pairwise alignment tutorial http://docs.seqan.de/seqan/3-master-user/tutorial_pairwise_alignment.html but instead of
std::vector<dna4>I use ranges. The code doesn't compile when I setseqan3::align_cfg::result{seqan3::with_alignment}.How to repeat the problem
The Code below reproduces the error.
If you remove
seqan3::align_cfg::result{seqan3::with_alignment}it compiles.If instead of a range I just use a
std::vector<dna4>for s2 it also compiles.Expected behaviour
I expect align_pairwise to work with ranges in all configurations.
Actual behaviour
I fails with a compile error:
The text was updated successfully, but these errors were encountered: