First draft of index module #292

cpockrandt · 2018-07-06T10:58:11Z

This is work in progress, don't merge ~~or review~~ yet :)

cpockrandt · 2018-08-06T18:14:32Z

Feel free to start reviewing the first draft :) Documentation will follow. Bidirectional index will follow after the review :)

eseiler

Looks good so far!

You may want to rebase on master, because the range concept moved (that's why the tests aren't compiling).

Otherwise some format and docs(where already existing) stuff and mainly questions for understanding.

eseiler · 2018-08-07T04:04:25Z

include/seqan3/index/concept.hpp

+
+#include <seqan3/alphabet/nucleotide/dna4.hpp>
+#include <seqan3/core/metafunction/range.hpp>
+#include <seqan3/range/concept.hpp>


Is now <seqan3/std/concept/range.hpp>

eseiler · 2018-08-07T04:04:52Z

include/seqan3/index/concept.hpp

+//                 SeqAn - The Library for Sequence Analysis
+// ============================================================================
+//
+// Copyright (c) 2006-2017, Knut Reinert & Freie Universitaet Berlin


2018

Also for the other files

eseiler · 2018-08-07T04:06:23Z

include/seqan3/index/concept.hpp

+// DAMAGE.
+//
+// ============================================================================
+// Author: Christopher Pockrandt <christopher.pockrandt AT fu-berlin.de>


We use doxygen for author information:

/*!\file

\brief ....

\author ...
*/

Also for the other files

include/seqan3/index/concept.hpp

+    requires requires (t index, std::vector<dna4> const & text) { { t(t(text)) } };
+    requires requires (t index, std::vector<dna4> const & text) { { index = t(text) } };
+
+    requires requires (t index, const std::vector<dna4> & text) { { t(text) } };


include/seqan3/index/all.hpp

@@ -0,0 +1,4 @@
+#pragma once


eseiler · 2018-08-07T05:07:16Z

include/seqan3/index/fm_index_iterator.hpp

+        typename index_type::comp_char_type c = node.last_char + 1;
+        typename index_type::size_type _lb, _rb;
+
+        while (c < index->m_index.sigma && !sdsl::backward_search(index->m_index, parent_lb, parent_rb, index->m_index.comp2char[c], _lb, _rb))


Line length

eseiler · 2018-08-07T05:10:43Z

test/unit/index/fm_index_iterator_test.cpp

+        EXPECT_EQ(it, iterator_type(sa));
+    }
+
+    // search a char that does not occur in the text (some rank that is neither the smallest nor the highest smallest occurring in text)


Line length

eseiler · 2018-08-07T05:15:08Z

test/unit/index/fm_index_iterator_test.cpp

+    text_type text {"ACGACG"_dna4};
+    index_type sa{text};
+
+    // custom constructor


I would split all the tests into parts that test one functionality.
I.e. one test for each constructor, one test for each function.

Disagree 😄

I don't see any need to create a separate test case for each constructor. We could replace testing the default constructors using the metafunctions.

edit: i'll split the tests in basic_search

eseiler · 2018-08-07T05:19:55Z

test/unit/index/fm_index_iterator_test.cpp

+
+#include <seqan3/index/fm_index.hpp>
+
+#include <seqan3/alphabet/nucleotide/dna4.hpp>


I think, we should test for all the alphabets we support?
The to_rank() functionality is already tested in the alphabet tests, but the fm_index still has stuff that is otherwise untested, i.e. sdsl::int_vector<8> tmp_text(text.size());, which should produce errors once we use UTF alphabets and have a character with rank > 255.

as mentioned before, we shouldn't support larger alphabets than for example amino acid alphabets. in that case the user should use indices for integer alphabets (where I currently don't see any application for in sequence analysis)

but we might enforce using byte-alphabets. any idea how to do that? we can't check for the underlying type (::char_type) since indices should work with types such as char, uint8_t as well

I think you can use std::is_same_v<underlying_rank<t>::type, uint8_t>, which should be implemented for all types t that satisfy the alphabet_concept (hence also uint8_t and char).
It's here and included with #include <seqan3/alphabet/concept.hpp>.

eseiler · 2018-08-07T05:20:36Z

test/unit/index/fm_index_test.cpp

+// TODO: EXPECT_EQ is not supported by sdsl
+// TODO: test bitcompressed vector for construction
+
+TEST(fm_index, ctr)


Same as above (alphabets, minimal test cases)

cpockrandt · 2018-08-07T12:52:22Z

@eseiler Thanks!

Tests won't compile anyway because it's blocked by #45 (sdsl-lite)

cpockrandt · 2018-08-07T15:58:47Z

Pretty much fixed all TODOs now that do not depend on external libraries.

Most important question for @h-2: should we make the constructors of the fm_index_iterator noexcept? Since they are often passed by value, I understand it might be faster avoiding exception handling at all.

eseiler

I like the docs so far 👍

include/seqan3/index/all.hpp

+// OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+// DAMAGE.
+//
+


eseiler · 2018-08-08T05:43:57Z

include/seqan3/index/all.hpp

+  * mappers, assemblers or protein search tools. There are currently two major kind of indices: FM indices and
+  * k-mer indices (also known as q-gram indices).
+  *
+  * Besides searching the index yourself using the iterator interfaces, SeqAn3 also provides a very powerful


[...] searching by yourself [...]

(I think both versions are fine, but with 'by' I find it easier to read.)

eseiler · 2018-08-08T05:44:23Z

include/seqan3/index/all.hpp

+  *
+  * You can choose between unidirectional and bidirectional FM indices (which can be thought of suffix trees
+  * and affix trees, i.e. a combination of suffix and prefix trees being able to search a pattern from left to
+  * right, right to left and character by character in any arbitrary order). Rougly speaking bidirectional


eseiler · 2018-08-08T05:48:05Z

include/seqan3/index/all.hpp

+  * You can choose between unidirectional and bidirectional FM indices (which can be thought of suffix trees
+  * and affix trees, i.e. a combination of suffix and prefix trees being able to search a pattern from left to
+  * right, right to left and character by character in any arbitrary order). Rougly speaking bidirectional
+  * FM indices are more powerful for approximate string matching for the cost of a higher space consumption


[...] string matching at the cost of [...]

eseiler · 2018-08-08T05:50:39Z

include/seqan3/index/all.hpp

+  * and affix trees, i.e. a combination of suffix and prefix trees being able to search a pattern from left to
+  * right, right to left and character by character in any arbitrary order). Rougly speaking bidirectional
+  * FM indices are more powerful for approximate string matching for the cost of a higher space consumption
+  * (depending on the configuration between a factor of TODO and TODO).


I would switch the parts and say:

between a factor of TODO and TODO depending on the configuration

(I read at first that the configuration is between a factor of...; so it kinda breaks the read flow)

eseiler · 2018-08-08T06:27:51Z

include/seqan3/index/fm_index_iterator.hpp

+ *
+ * The iterator traverses the implicit suffix tree beginning at the root node. All methods
+ * modifying the iterator (e.g. going down an edge) return a `bool` value whether the operation was successful or not.
+ * In case of an unsuccessful operation the iterator remains unmodified, i.e. there an iterator can never be in an


[...] i.e. ~~there~~ an iterator [...]

eseiler · 2018-08-08T06:29:27Z

include/seqan3/index/fm_index_iterator.hpp

+ * The iterator traverses the implicit suffix tree beginning at the root node. All methods
+ * modifying the iterator (e.g. going down an edge) return a `bool` value whether the operation was successful or not.
+ * In case of an unsuccessful operation the iterator remains unmodified, i.e. there an iterator can never be in an
+ * invalid state except default constructed iterators that are always invalid. E.g. going down an edge while the


[...] E.g., going down [...]

eseiler · 2018-08-08T06:31:23Z

include/seqan3/index/fm_index_iterator.hpp

+ * iterator is in a leaf node, will return `false` and will remain in that leaf node.
+ *
+ * The asymptotic running times for using the iterator depend on the SDSL index configuration. To determine the exact
+ * running times, you have to look up the running times of the used traits (configuration) additionally.


you have to additionally look up the running times of the used traits (configuration) ~~additionally~~.

eseiler · 2018-08-08T06:34:52Z

include/seqan3/index/fm_index_iterator.hpp

+     *          sequence does not occur in the indexed text).
+     *
+     * If the going down the edges fails in the middle of the sequence, all previous computations are rewound to restore
+     * the iterators state before calling this method.


[...] the iterator's state [...]

eseiler · 2018-08-08T06:38:36Z

include/seqan3/index/fm_index_iterator.hpp

+        return depth() == 0;
+    }
+
+    // TODO: naming? maybe dereference-operator instead?


I like the idea of using a dereference operator

Do you think we should offer a "named" method as well?

I'm not sure, for me it's clear what I expect from dereferencing the iterator, but a named method might be more verbose?

Do you have a suggestion? I'm not really happy with path_label() 🤔

get_path, path_representation (which I find quite long) ?
I actually find path_label catchy

h-2

Please make the test cases minimal as suggested by enrico
Concerning the module structure I would propose to have index in the search module as both things cannot be used without the other, lets discuss the strucutre together!

h-2 · 2018-08-08T14:46:22Z

include/seqan3/index/all.hpp

+  * and affix trees, i.e. a combination of suffix and prefix trees being able to search a pattern from left to
+  * right, right to left and character by character in any arbitrary order). Roughly speaking bidirectional
+  * FM indices are more powerful for approximate string matching at the cost of a higher space consumption
+  * (between a factor of TODO and TODO depending on the configuration).


in the documentation use \TODO for TODOs as that makes them appear extra on:
http://docs.seqan.de/seqan/3.0.0-master-dev/todo.html

h-2 · 2018-08-08T14:49:37Z

include/seqan3/index/all.hpp

+  * FM indices are more powerful for approximate string matching at the cost of a higher space consumption
+  * (between a factor of TODO and TODO depending on the configuration).
+  *
+  * The FM indices are based on the SDSL (succinct data structure library). You are able to specify the


h-2 · 2018-08-08T14:52:10Z

include/seqan3/index/concept.hpp

+//!\cond
+template <typename t>
+concept bool fm_index_traits_concept = requires (t v,
+    typename t::sdsl_index_type::size_type lb,


h-2 · 2018-08-08T14:53:53Z

include/seqan3/index/concept.hpp

+    requires requires (t index, std::vector<dna4> const & text) { { t(t(text)) } };
+    requires requires (t index, std::vector<dna4> const & text) { { index = t(text) } };
+
+    requires requires (t index, const std::vector<dna4> & text) { { t(text) } };


semiregular concept likely includes everything you want

include/seqan3/index/fm_index.hpp

+     *
+     * ### Complexity
+     *
+     * At least linear.


h-2 · 2018-08-08T15:11:11Z

include/seqan3/index/fm_index_iterator.hpp

+public:
+
+    // TODO(h-2):
+    // * shouldn't all these constructors be noexcept? construction will happen a lot in recursions


h-2 · 2018-08-08T15:13:39Z

include/seqan3/index/fm_index_iterator.hpp

+        assert(index != nullptr);
+
+        // parent_lb/parent_rb might be uninitialized in a root node
+        return node == rhs.node && ((parent_lb == rhs.parent_lb && parent_rb == rhs.parent_rb) || depth() == 0);


check depth == 0 before the other condition to prevent read from unitialised memory

h-2 · 2018-08-08T15:16:48Z

include/seqan3/index/fm_index_iterator.hpp

+     *
+     * No-throw guarantee.
+     */
+    template <range_concept pattern_t>


check actual contraints; at least input_range, but also sized if you use size()

h-2 · 2018-08-08T15:18:06Z

include/seqan3/index/fm_index_iterator.hpp

+    	for (typename index_t::size_type i = 0; i < occ.size(); ++i) {
+    		occ[i] = offset() - index->m_index[node.lb + i];
+    	}
+    	return occ;


h-2 · 2018-08-08T15:18:15Z

include/seqan3/index/fm_index_iterator.hpp

+        assert(index != nullptr);
+
+    	std::vector<size_type> occ(count());
+    	for (typename index_t::size_type i = 0; i < occ.size(); ++i) {


h-2

minor stuff

h-2 · 2018-09-12T11:23:08Z

test/snippet/index/bi_fm_index_iterator.cpp

+    {
+        //! [cycle]
+        std::vector<dna4> genome{"GAATTAATGAAC"_dna4};
+        bi_fm_index<std::vector<dna4>> index{genome}; // build the index


you should be able to omit the template parameter

h-2 · 2018-09-12T11:25:19Z

test/snippet/index/bi_fm_index_iterator.cpp

+        std::cout << it.query() << '\n'; // outputs "AAC"
+        std::cout << it.last_char() << '\n'; // outputs 'C'
+
+        // it.cycle_front(); // undefined behaviour! only cycle_back() is allowed after extend_right()


Would it be better to throw? Does one cycle often in algorithms?

I think we already discussed this and decided to make it undefined behaviour. Yes, pretty much all the time when you do approximate string matching

h-2 · 2018-09-12T11:25:29Z

test/snippet/index/bi_fm_index_iterator.cpp

+        it.cycle_front(); // search the sequence "TAAT"
+        std::cout << it.query() << '\n'; // outputs "TAAT"
+        std::cout << it.last_char() << '\n'; // outputs 'T'
+        //! [cycle]


nice examples!

h-2 · 2018-09-12T11:26:04Z

test/snippet/index/bi_fm_index_iterator.cpp

+
+int main()
+{
+    {


in general, don't indent top-level(s) in the snippets, because it will also indent in the documentation.

h-2 · 2018-09-12T11:29:02Z

test/snippet/index/bi_fm_index_iterator.cpp

+        bi_fm_index<std::vector<dna4>> index{genome}; // build the index
+
+        auto it = index.begin(); // create an iterator
+        it.extend_left("CAA"_dna4); // search the sequence "AAC"


I thought you said one doesn't have to reverse the sequences mentally?

That's an open discussion see

seqan3/include/seqan3/index/bi_fm_index_iterator.hpp

Line 628 in 41109a6

// TODO(h-2, eseiler and cpockrandt): should we iterate from left to right or right to left?

h-2 · 2018-09-12T11:29:52Z

test/snippet/index/bi_fm_index_iterator.cpp

+        //! [cycle]
+    }
+
+    std::cout << "-------------------------\n";


why are you printing this?

In case a user wants to compile the snippet it's hard to see where the output of the different blocks start and end. But I'll make a more meaningful output instead of ------ 😃

h-2 · 2018-09-12T11:31:21Z

test/snippet/index/bi_fm_index_iterator.cpp

+        // Undefined behaviour! Cannot be called on the forward iterator if the last extension on the bidirectional
+        // iterator was to the left:
+        // it.cycle_back();
+        // std::cout << it.last_char() << '\n';


last_char() also causes UB?

Yes. If you do an extension to the left, than you can only do cycle_front() and last_char() will return the leftmost character. When you now construct a unidirectional fwd iterator (that goes to the right on the original text), last_char() will give you the leftmost char even though last_char() on a unidirectional iterator is supposed to give you always the rightmost character. That's why it's undefined

We could put a check in debug mode into the unidirectional iterator (when it is constructed from a bidirectional one) as we did it in the bidirectional iterator. I'll put this on my TODO list to work on after GCB.

h-2 · 2018-09-12T11:32:08Z

test/snippet/index/bi_fm_index_iterator.cpp

+        uni_it.extend_right(dna4::G); // search the sequence "AACG"
+        std::cout << uni_it.query() << '\n'; // outputs "AACG"
+        std::cout << uni_it.last_char() << '\n'; // outputs 'G'
+        uni_it.cycle_back(); // returns false since there is not sequence "AACT" in the text.


test/unit/index/fm_index_iterator_test.cpp

eseiler · 2018-09-13T05:57:50Z

include/seqan3/search/fm_index/bi_fm_index.hpp

+
+#pragma once
+
+#include <filesystem>


Can you replace this with

#include <seqan3/io/filesystem.hpp>

It resolves to std::experimental::filesystem or std::filesystem - depending on what is available and puts it into the seqan3::filesystem namespace

And then replace

std::filesystem::

with

filesystem::

eseiler · 2018-09-13T05:58:31Z

include/seqan3/search/fm_index/fm_index.hpp

+
+#pragma once
+
+#include <filesystem>


Same here, replace with

#include <seqan3/io/filesystem.hpp>

h-2

minor stuff

h-2 · 2018-09-13T14:19:37Z

include/seqan3/index/bi_fm_index.hpp

+//!\cond
+    requires alphabet_concept<innermost_value_type_t<text_t>>
+        && std::is_same_v<typename underlying_rank<innermost_value_type_t<text_t>>::type, uint8_t>
+//!\endcond


💅 underlying_rank_t<innermost_value_type_t<text_t>

include/seqan3/search/fm_index/bi_fm_index.hpp

h-2 · 2018-09-13T14:27:05Z

include/seqan3/search/fm_index/bi_fm_index_iterator.hpp

+ *
+ * The asymptotic running times for using the iterator depend on the SDSL index configuration. To determine the exact
+ * running times, you have to additionally look up the running times of the used traits (configuration).
+ */


please implement my previous suggestion regarding the "iterator" disclaimer.

edit: I am starting to be more and more convinced that we actually should not call them iterators, but maybe cursors or something like that...

I'll think about it during the weekend 🤔

Given, that the iterator concept requires dereferencing and incrementing operations, and that in fact an index is not a range (is it?), I would also argue for something different. Cursor sounds like a perfect candidate. In fact the ranges library offers a cursor interface and also a function called cursor_begin(), cursor_end() etc.. I would really prefer this interface.

include/seqan3/search/fm_index/fm_index_iterator.hpp

test/include/seqan3/test/comparison.hpp

test/snippet/index/bi_fm_index_iterator.cpp

test/unit/index/bi_fm_index_iterator_test.cpp

h-2 · 2018-09-13T15:01:49Z

by the way, can you index collections now?

rrahn

Some first look. I will continue tomorrow.

rrahn · 2018-09-13T17:17:49Z

include/seqan3/search/all.hpp

+  * Searching is a key component in many sequence analysis tools. The search module is a powerful and easy way to search
+  * sequences in a large text or an arbitrary nested collection of texts. When it comes to searching, indices are a core
+  * component for searching large amounts of data and are used for tools such as read mappers, assemblers or protein
+  * search tools. There are currently two major kind of indices: FM indices and k-mer indices (also known as q-gram


Not sure, if FM-index is more generic than Suffix Array, if you talk about two major kinds?

include/seqan3/search/all.hpp

rrahn · 2018-09-13T17:22:55Z

include/seqan3/search/fm_index/bi_fm_index.hpp

+
+public:
+
+    /*!\name Member types


I think this group is superfluous

include/seqan3/search/fm_index/bi_fm_index.hpp

include/seqan3/search/fm_index/bi_fm_index_iterator.hpp

rrahn · 2018-09-13T19:24:39Z

include/seqan3/search/fm_index/bi_fm_index_iterator.hpp

+ *
+ * The asymptotic running times for using the iterator depend on the SDSL index configuration. To determine the exact
+ * running times, you have to additionally look up the running times of the used traits (configuration).
+ */


Given, that the iterator concept requires dereferencing and incrementing operations, and that in fact an index is not a range (is it?), I would also argue for something different. Cursor sounds like a perfect candidate. In fact the ranges library offers a cursor interface and also a function called cursor_begin(), cursor_end() etc.. I would really prefer this interface.

… to failure)

cpockrandt force-pushed the feature/index branch 6 times, most recently from 1d28372 to 336e051 Compare August 6, 2018 18:09

cpockrandt requested review from h-2 and eseiler August 6, 2018 18:10

cpockrandt assigned h-2 Aug 6, 2018

cpockrandt force-pushed the feature/index branch from 336e051 to 65f041e Compare August 6, 2018 18:12

eseiler requested changes Aug 7, 2018

View reviewed changes

cpockrandt force-pushed the feature/index branch from 65f041e to 9287193 Compare August 7, 2018 12:45

cpockrandt force-pushed the feature/index branch 3 times, most recently from df17567 to db651eb Compare August 7, 2018 15:24

cpockrandt force-pushed the feature/index branch 5 times, most recently from 6e9748c to 8e8099b Compare August 8, 2018 00:14

eseiler requested changes Aug 8, 2018

View reviewed changes

cpockrandt force-pushed the feature/index branch 4 times, most recently from eb6ca59 to f9a7ab3 Compare August 8, 2018 14:42

h-2 reviewed Aug 8, 2018

View reviewed changes

cpockrandt force-pushed the feature/index branch from f9a7ab3 to 341bed4 Compare August 8, 2018 17:30

h-2 requested changes Sep 12, 2018

View reviewed changes

cpockrandt force-pushed the feature/index branch 4 times, most recently from e212324 to 6943a45 Compare September 13, 2018 00:33

eseiler reviewed Sep 13, 2018

View reviewed changes

cpockrandt force-pushed the feature/index branch 3 times, most recently from ae530c0 to be2d41a Compare September 13, 2018 12:53

h-2 requested changes Sep 13, 2018

View reviewed changes

rrahn reviewed Sep 13, 2018

View reviewed changes

cpockrandt force-pushed the feature/index branch 12 times, most recently from d33774a to cb72be4 Compare September 17, 2018 12:34

cpockrandt added 3 commits September 18, 2018 01:43

[FEATURE, TEST] Unidirectional FM index and iterator

08f4f47

[FEATURE, TEST] Bidirectional FM index and iterator

c7225a3

[FIX] Deactive header tests for search module temporarily (sdsl leads…

0514ee3

… to failure)

cpockrandt force-pushed the feature/index branch from cb72be4 to 0514ee3 Compare September 17, 2018 23:44

h-2 approved these changes Sep 18, 2018

View reviewed changes

h-2 merged commit 19f4ed0 into seqan:master Sep 18, 2018


		#include <seqan3/index/fm_index.hpp>

		#include <seqan3/alphabet/nucleotide/dna4.hpp>

First draft of index module #292

First draft of index module #292

Conversation

cpockrandt commented Jul 6, 2018 • edited Loading

cpockrandt commented Aug 6, 2018

eseiler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpockrandt Aug 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eseiler Aug 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpockrandt commented Aug 7, 2018

cpockrandt commented Aug 7, 2018 • edited Loading

eseiler left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpockrandt Sep 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

h-2 commented Sep 13, 2018

rrahn left a comment

Choose a reason for hiding this comment

cpockrandt commented Jul 6, 2018 •

edited

Loading

cpockrandt Aug 7, 2018 •

edited

Loading

eseiler Aug 7, 2018 •

edited

Loading

cpockrandt commented Aug 7, 2018 •

edited

Loading

cpockrandt Sep 13, 2018 •

edited

Loading