[MISC] Update to seqan 3.0.3 #173
Conversation
There was a problem hiding this comment.
Other than the two small comments, this all looks good! Thank you for working on this!
Before we merge, we need to compare the performance. If the results of the search are changed, it would also be good to know how they changed exactly (minor difference in few results or larger changes).
| // The return type of views::pos_transform | ||
| template <std::ranges::view urng_t, typename pos_transform_t, typename size_transform_t> | ||
| class view_pos_transform : public ranges::view_base | ||
| class view_pos_transform : public std::ranges::view_interface<view_pos_transform<urng_t, pos_transform_t, size_transform_t>> //public ranges::view_base |
There was a problem hiding this comment.
I am not against this change, but why was it necessary?
There was a problem hiding this comment.
I think this was from trying stuff out, I can probably just revert (or use std::ranges::view_base). Will check
| template <typename t, bool b1, bool b2> | ||
| inline constexpr bool is_new_range<seqan3::detail::view_take<t, b1, b2>> = true; | ||
| template <typename t> | ||
| inline constexpr bool is_new_range<std::ranges::take_view<t>> = true; |
There was a problem hiding this comment.
This means is_new_range is specialised for the the standard take_view but not anymore for SeqAn's take_view. Shouldn't we have both?
There was a problem hiding this comment.
We can have both, but as far as I remember this is important because seqan3::views::slice is used somewhere. In seqan3 we now use the std views for slice, so we technically don't need the specialisation for seqan3.
Differences in few results, and only in the SIMD tests, Serial stayed the same. SIMD changes are due to fixes in SIMD alignment seqan/seqan#2417 |
Minor differences: 2% of the hits change, the new hit is always longer than previously reported with slightly lower percent identity. The end of the query/subject stay always the same, only the start changes. This matches the expectation we had from the problem we reported last year for SIMD which as been fixed in this PR I think. |
|
Merged this, thank you! Any necessary changes will be performed in our branch. |
The checksums for the tests changed because of some fixed bugs/new features:
The important change for lambda is that the bi_fm_index now samples fewer positions in its reverse fm_index (we only need to sample in one fm_index and we do it in the forward index). That also means that newly built indices are different from the old ones.
There were some fixes regarding SIMD alignment, hence all
fullSIMDtest results changed.Regarding CI updates:
In SeqAn3, we put most of the bash scripts into
.shfiles. This also means we can use those scripts whenever we have seqan3 as submodule. The scripts take care of ubuntu/macOS differences and try to be relatively fail-safe.