Skip to content
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

[FEATURE] Serialise index cursor #2048

Merged
merged 1 commit into from
Aug 31, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ Note that 3.1.0 will be the first API stable release and interfaces in this rele
([\#1853](https://github.com/seqan/seqan3/pull/1853)).
* Added `seqan3::search_cfg::on_result`, which allows providing a custom callback for the search algorithm
([\#2019](https://github.com/seqan/seqan3/pull/2019)).
* The `seqan3::fm_index_cursor` and `seqan3::bi_fm_index_cursor` can be serialised
([\#2048](https://github.com/seqan/seqan3/pull/2019)).

## API changes

Expand Down
23 changes: 23 additions & 0 deletions include/seqan3/search/fm_index/bi_fm_index_cursor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace seqan3
/*!\brief The SeqAn Bidirectional FM Index Cursor.
* \implements seqan3::bi_fm_index_cursor_specialisation
* \tparam index_t The type of the underlying index; must model seqan3::bi_fm_index_specialisation.
* \implements seqan3::cerealisable
* \details
*
* The cursor's interface provides searching a string both from left to right as well as from right to left in the
Expand Down Expand Up @@ -1022,6 +1023,28 @@ class bi_fm_index_cursor
return locate_result_value_type{sequence_rank-1, sequence_position};
});
}

/*!\cond DEV
* \brief Serialisation support function.
* \tparam archive_t Type of `archive`; must satisfy seqan3::cereal_archive.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* \tparam archive_t Type of `archive`; must satisfy seqan3::cereal_archive.
* \tparam archive_t Type of `archive`; must model seqan3::cereal_archive.

💅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use satifsy in every serialisation method (~12 times). Can I do this as separated PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

* \param archive The archive being serialised from/to.
*
* \attention These functions are never called directly, see \ref serialisation for more details.
*/
template <cereal_archive archive_t>
void CEREAL_SERIALIZE_FUNCTION_NAME(archive_t & archive)
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this function is not visible to the user, we should mention this in the class documentation. I think we might just used \imlements cerealisable or something similar.. Can you check other classes?

Also Can you add a changelog entry, since this seems to be of interest to users :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but then I would also rebase this against the release branch?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this function is not visible to the user, we should mention this in the class documentation. I think we might just used \imlements cerealisable or something similar.. Can you check other classes?

Yes, it would be great to see directly in the documentation which classes can be serialized with cereal, I had to study the code to see that neither cerealisation is possible, nor I can access the members on my own. I guess adding something like \implements cerealisable would be perfectly suitable for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the documentation is only reflected in the inheritance diagram as far as I can see, https://docs.seqan.de/seqan/3-master-user/interfaceseqan3_1_1cerealisable.html

We should rework this

archive(fwd_lb);
archive(fwd_rb);
archive(rev_lb);
archive(rev_rb);
archive(sigma);
archive(parent_lb);
archive(parent_rb);
archive(_last_char);
archive(depth);
}
//!\endcond
};

//!\}
Expand Down
20 changes: 19 additions & 1 deletion include/seqan3/search/fm_index/detail/fm_index_cursor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include <tuple>
#include <type_traits>

#include <seqan3/core/platform.hpp>
#include <seqan3/core/concept/cereal.hpp>

namespace seqan3::detail
{
Expand All @@ -27,6 +27,7 @@ namespace seqan3::detail
/*!\brief Internal representation of the node of an FM index cursor.
* \ingroup fm_index
* \tparam index_t The type of the underlying index; must satisfy seqan3::fm_index_specialisation.
* \implements seqan3::cerealisable
*/
template <typename index_t>
struct fm_index_cursor_node
Expand Down Expand Up @@ -61,6 +62,23 @@ struct fm_index_cursor_node
{
return !(*this == rhs);
}

/*!\cond DEV
* \brief Serialisation support function.
* \tparam archive_t Type of `archive`; must satisfy seqan3::cereal_archive.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* \tparam archive_t Type of `archive`; must satisfy seqan3::cereal_archive.
* \tparam archive_t Type of `archive`; must model seqan3::cereal_archive.

* \param archive The archive being serialised from/to.
*
* \attention These functions are never called directly, see \ref serialisation for more details.
*/
template <cereal_archive archive_t>
void CEREAL_SERIALIZE_FUNCTION_NAME(archive_t & archive)
{
archive(lb);
archive(rb);
archive(depth);
archive(last_char);
}
//!\endcond
};

// std::tuple get_suffix_array_range(fm_index_cursor<index_t> const & it)
Expand Down
18 changes: 18 additions & 0 deletions include/seqan3/search/fm_index/fm_index_cursor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace seqan3
/*!\brief The SeqAn FM Index Cursor.
* \implements seqan3::fm_index_cursor_specialisation
* \tparam index_t The type of the underlying index; must model seqan3::fm_index_specialisation.
* \implements seqan3::cerealisable
* \details
*
* The cursor's interface provides searching a string from left to right in the indexed text.
Expand Down Expand Up @@ -621,6 +622,23 @@ class fm_index_cursor
return locate_result_value_type{sequence_rank - 1, sequence_position};
});
}

/*!\cond DEV
* \brief Serialisation support function.
* \tparam archive_t Type of `archive`; must satisfy seqan3::cereal_archive.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* \tparam archive_t Type of `archive`; must satisfy seqan3::cereal_archive.
* \tparam archive_t Type of `archive`; must model seqan3::cereal_archive.

* \param archive The archive being serialised from/to.
*
* \attention These functions are never called directly, see \ref serialisation for more details.
*/
template <cereal_archive archive_t>
void CEREAL_SERIALIZE_FUNCTION_NAME(archive_t & archive)
{
archive(parent_lb);
archive(parent_rb);
archive(node);
archive(sigma);
}
//!\endcond
};

//!\}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <seqan3/core/detail/debug_stream_tuple.hpp>
#include <seqan3/range/views/slice.hpp>
#include <seqan3/search/fm_index/bi_fm_index_cursor.hpp>
#include <seqan3/test/cereal.hpp>
#include <seqan3/test/expect_range_eq.hpp>

#include "../helper.hpp"
Expand Down Expand Up @@ -220,5 +221,16 @@ TYPED_TEST_P(bi_fm_index_cursor_collection_test, extend_const_char_pointer)
}
}

TYPED_TEST_P(bi_fm_index_cursor_collection_test, serialisation)
{
typename TypeParam::index_type bi_fm{this->text_col2};

TypeParam it = TypeParam(bi_fm);
it.extend_left(seqan3::views::slice(this->text, 1, 3));

seqan3::test::do_serialisation(it);
}

REGISTER_TYPED_TEST_SUITE_P(bi_fm_index_cursor_collection_test, cursor, extend, extend_char, extend_range,
extend_and_cycle, extend_range_and_cycle, to_fwd_cursor, extend_const_char_pointer);
extend_and_cycle, extend_range_and_cycle, to_fwd_cursor, extend_const_char_pointer,
serialisation);
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <seqan3/core/detail/debug_stream_tuple.hpp>
#include <seqan3/range/views/slice.hpp>
#include <seqan3/search/fm_index/bi_fm_index_cursor.hpp>
#include <seqan3/test/cereal.hpp>
#include <seqan3/test/expect_range_eq.hpp>

#include "../helper.hpp"
Expand Down Expand Up @@ -184,5 +185,15 @@ TYPED_TEST_P(bi_fm_index_cursor_test, to_fwd_cursor)
}
}

TYPED_TEST_P(bi_fm_index_cursor_test, serialisation)
{
typename TypeParam::index_type bi_fm{this->text};

TypeParam it(bi_fm);
it.extend_left(seqan3::views::slice(this->text, 1, 3));

seqan3::test::do_serialisation(it);
}

REGISTER_TYPED_TEST_SUITE_P(bi_fm_index_cursor_test, cursor, extend, extend_char, extend_range, extend_and_cycle,
extend_range_and_cycle, to_fwd_cursor);
extend_range_and_cycle, to_fwd_cursor, serialisation);
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <seqan3/range/views/slice.hpp>
#include <seqan3/search/fm_index/concept.hpp>
#include <seqan3/std/algorithm>
#include <seqan3/test/cereal.hpp>
#include <seqan3/test/expect_range_eq.hpp>

#include <sdsl/csa_wt.hpp>
Expand Down Expand Up @@ -348,7 +349,17 @@ TYPED_TEST_P(fm_index_cursor_collection_test, concept_check)
EXPECT_TRUE(seqan3::fm_index_cursor_specialisation<TypeParam>);
}

TYPED_TEST_P(fm_index_cursor_collection_test, serialisation)
{
typename TypeParam::index_type fm{this->text_col4};

TypeParam it(fm);
it.extend_right(seqan3::views::slice(this->text1, 0, 3));

seqan3::test::do_serialisation(it);
}

REGISTER_TYPED_TEST_SUITE_P(fm_index_cursor_collection_test, ctr, begin, extend_right_range,
extend_right_range_empty_text, extend_right_char, extend_right_range_and_cycle,
extend_right_char_and_cycle, extend_right_and_cycle, query, last_rank, incomplete_alphabet,
lazy_locate, extend_const_char_pointer, concept_check);
lazy_locate, extend_const_char_pointer, concept_check, serialisation);
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <seqan3/range/views/slice.hpp>
#include <seqan3/search/fm_index/concept.hpp>
#include <seqan3/std/algorithm>
#include <seqan3/test/cereal.hpp>
#include <seqan3/test/expect_range_eq.hpp>

#include <sdsl/csa_wt.hpp>
Expand Down Expand Up @@ -300,6 +301,16 @@ TYPED_TEST_P(fm_index_cursor_test, concept_check)
EXPECT_TRUE(seqan3::fm_index_cursor_specialisation<TypeParam>);
}

TYPED_TEST_P(fm_index_cursor_test, serialisation)
{
typename TypeParam::index_type fm{this->text1};

TypeParam it = TypeParam(fm);
it.extend_right(seqan3::views::slice(this->text1, 0, 3));

seqan3::test::do_serialisation(it);
}

REGISTER_TYPED_TEST_SUITE_P(fm_index_cursor_test, ctr, begin, extend_right_range, extend_right_char,
extend_right_range_and_cycle, extend_right_char_and_cycle, extend_right_and_cycle, query,
last_rank, incomplete_alphabet, lazy_locate, concept_check);
last_rank, incomplete_alphabet, lazy_locate, concept_check, serialisation);