ARROW-13064: [C++] Implement select ('case when') function for fixed-width types #10557

lidavidm · 2021-06-18T20:06:13Z

This doesn't support variable-width types (e.g. strings) as the implementation here is columnwise. I will work on those separately (they require a rowwise implementation).

Also fixes a small bug in the CommonNumericType implementation (I noticed uint8 was getting promoted to int8).

github-actions · 2021-06-18T20:06:30Z

https://issues.apache.org/jira/browse/ARROW-13064

lidavidm · 2021-06-25T17:36:02Z

Note there's some code here for fixed-width types that duplicates what's in ARROW-9430/#10412. They should probably get unified.

jorisvandenbossche · 2021-06-29T09:21:42Z

To start my usual round of name bike-shedding ;), I would propose to not call this "select", to avoid the confusion with SQL's SELECT / dplyr's select() function (I think the "select" name only comes from np.select?)
Given this is for SQL's CASE, maybe something like "select_case" or "case_when" (or "choose_case", or ..)?

lidavidm · 2021-06-29T12:10:31Z

case_when sounds good to me. Or perhaps switch_case by analogy to if_else.

pitrou · 2021-06-29T15:39:55Z

Ok for case_when, another possibility is cond (like in Lisp ;-)).

lidavidm · 2021-06-29T19:58:35Z

Renamed to case_when.

pitrou · 2021-06-30T14:52:35Z

It looks like this needs rebasing.

lidavidm · 2021-06-30T15:01:34Z

Thanks for the heads up, rebased.

pitrou

Very nice, thank you. Do you want to add a couple benchmarks for this?

pitrou · 2021-06-30T16:38:22Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

Nit, but would be nice to normalize includes here. I think we normally use #include "arrow/..." for intra-Arrow inclusions.

pitrou · 2021-06-30T16:39:29Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

Instead use ValidateOutput from kernels/test_util.h. It will also check that no data is left uninitialized.

pitrou · 2021-06-30T16:40:59Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

I wonder if this should be moved to kernels/test_util.{h,cc} instead?

Also worth noting: this seems to be very similar in function to CheckWithDifferentShapes. I think it'd be worthwhile to unify these two and promote it to test_util.h so it can be reused by more varargs scalar kernels

It's also somewhat overlapping with CheckScalar now that I look at it. I'll take a look and see if I can't consolidate all three. (I may do so in a different PR until we decide what to do with the implementation here, if we want to split it up into separate 'case' and 'when' functions or not as suggested.)

pitrou · 2021-06-30T16:43:26Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

Also {scalar_false, values2, scalar_true, values1} -> values1?

pitrou · 2021-06-30T16:44:16Z

cpp/src/arrow/compute/api_scalar.h

So a condition being null is the same as being false, right?

Yes, null is the same as false.

pitrou · 2021-06-30T17:10:20Z

cpp/src/arrow/compute/kernels/scalar_if_else.cc

Given the definition of ReplaceTypes, this check doesn't seem necessary?
cc @bkietz for a second opinion.

It seems to me we could restructure "case_when" and yield a less daunting interface:

case( when(cond_0, value_0), when(cond_1, value_1), value_else )

Where when masks slots with null wherever its condition is not true and case is only a variadic coalescing function (takes the first non null).

We'd be allocating a new null bitmap on each call to when which is not ideal. However typing is far clearer (when(cond: Boolean, value: T): T, case(...values: T): T) and case/when can be independently unit tested.

@pitrou what do you think?

Note: variadic coalesce is independently useful https://issues.apache.org/jira/browse/ARROW-13136

Another way to transpose this would be:

case( first_true_in(cond_0, cond1), value0, value1, value_else )

Where first_true_in returns the index of the first true and case's first argument is an index into the rest of its arguments:

assert first_true_in([True, False, False], [True, True, False]) == [0, 1, None] assert case([0, 1, None], 'a b c'.split(), 'd e f'.split(), 'x y z'.split()) == 'a e z'.split()

In the second example, case would essentially be 'choose' (ARROW-13220). I'm curious which would be faster; the coalesce PR is implemented columnwise (for fixed-width types) and tries to do block copies, while the 'choose' PR I'm looking at is necessarily rowwise. Probably depends on the distribution of the inputs.

Might the masking 'when' kernel and 'first_true_in' be useful kernels to have anyways?

SGTM, thanks!

I think that

case( when(cond_0, value_0), when(cond_1, value_1), value_else )

is not necessarily identical to case_when(cond_0, value_0, cond_1, value_1, value_else) as done in this PR, when it comes to nulls in the values?

For example, if you have a null in value_0, it should be chosen if the corresponding bit of cond_0 is True. However, if you implement that with a masking when and then coalesce (taking the first non null), it would not pick the null value.

Hmm, you're right…

We can still try the 'choose' based version though I would expect that to be slower since it works rowwise.

I put up PRs for both choose: #10642 and coalesce: #10608 (though as you point out, we can't quite use coalesce for this).

A quick check with choose shows that it's only about 2/3 as fast:

CaseWhenBench32/1048576/0 48183534 ns 48183323 ns 13 bytes_per_second=334.659M/s CaseWhenBench64/1048576/0 48610718 ns 48610432 ns 13 bytes_per_second=660.866M/s CaseWhenBench32/1048576/99 48226819 ns 48226677 ns 15 bytes_per_second=334.327M/s CaseWhenBench64/1048576/99 48891957 ns 48892172 ns 13 bytes_per_second=656.996M/s

though note we have to call both first_true_in, then fill_null, to get the 'else' behavior which doesn't help.

pitrou · 2021-06-30T17:12:54Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

Somewhere, you should also test what happens when case_when is called with zero arguments.

pitrou · 2021-06-30T17:15:35Z

cpp/src/arrow/compute/kernels/scalar_if_else.cc

Instead of uint8_t* out_values, you may want this to take a ArrayData* out, since you'll need it for non-fixed-width types?

For non-fixed-width types, it might be simpler to handle it with a builder and AppendScalar?

pitrou · 2021-06-30T17:16:05Z

cpp/src/arrow/compute/kernels/scalar_if_else.cc

This seems a bit excessive?

This seems to be faster as written, oddly.

Before:

----------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations UserCounters... ----------------------------------------------------------------------------------------------- CaseWhenBench32/1048576/0 31933112 ns 31932368 ns 22 bytes_per_second=504.974M/s CaseWhenBench64/1048576/0 33170481 ns 33168735 ns 21 bytes_per_second=968.533M/s CaseWhenBench32/1048576/99 32487300 ns 32487411 ns 21 bytes_per_second=496.299M/s CaseWhenBench64/1048576/99 33682029 ns 33680901 ns 21 bytes_per_second=953.715M/s CaseWhenBench32Contiguous/1048576/0 7255445 ns 7255387 ns 96 bytes_per_second=1.632G/s CaseWhenBench64Contiguous/1048576/0 7932437 ns 7932171 ns 88 bytes_per_second=2.97013G/s CaseWhenBench32Contiguous/1048576/99 7526742 ns 7526709 ns 92 bytes_per_second=1.57303G/s CaseWhenBench64Contiguous/1048576/99 8172498 ns 8172239 ns 83 bytes_per_second=2.88261G/s

After:

----------------------------------------------------------------------------------------------- Benchmark Time CPU Iterations UserCounters... ----------------------------------------------------------------------------------------------- CaseWhenBench32/1048576/0 44166172 ns 44165634 ns 16 bytes_per_second=365.103M/s CaseWhenBench64/1048576/0 44605356 ns 44603995 ns 16 bytes_per_second=720.227M/s CaseWhenBench32/1048576/99 44867670 ns 44867051 ns 16 bytes_per_second=359.361M/s CaseWhenBench64/1048576/99 45077818 ns 45076721 ns 15 bytes_per_second=712.607M/s CaseWhenBench32Contiguous/1048576/0 17757494 ns 17757271 ns 39 bytes_per_second=682.819M/s CaseWhenBench64Contiguous/1048576/0 18236327 ns 18235892 ns 38 bytes_per_second=1.29193G/s CaseWhenBench32Contiguous/1048576/99 15051281 ns 15051008 ns 39 bytes_per_second=805.518M/s CaseWhenBench64Contiguous/1048576/99 15504081 ns 15503998 ns 45 bytes_per_second=1.51944G/s

pitrou · 2021-06-30T17:19:54Z

cpp/src/arrow/compute/kernels/scalar_if_else.cc

You can use GetNullCount() I think?

bkietz · 2021-06-30T18:44:45Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

This doesn't seem significantly different from CheckDispatchBest

bkietz · 2021-06-30T18:46:23Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

I agree; there's too much indirection here to see what's actually being tested. Please inline this some

bkietz · 2021-06-30T18:48:19Z

cpp/src/arrow/compute/kernels/scalar_if_else_test.cc

Also worth noting: this seems to be very similar in function to CheckWithDifferentShapes. I think it'd be worthwhile to unify these two and promote it to test_util.h so it can be reused by more varargs scalar kernels

bkietz · 2021-06-30T19:18:59Z

cpp/src/arrow/compute/kernels/scalar_if_else.cc

std::copy allows the ranges to overlap (unlike memcpy), so for simple pointers like this it gets inlined to memmove. Memmove can be slower (or faster) than memcpy; depends on your libc. It shouldn't differ by a very wide margin though so I don't think it's necessary to avoid std::copy

bkietz · 2021-06-30T19:30:46Z

cpp/src/arrow/compute/kernels/scalar_if_else.cc

It seems to me we could restructure "case_when" and yield a less daunting interface:

case( when(cond_0, value_0), when(cond_1, value_1), value_else )

Where when masks slots with null wherever its condition is not true and case is only a variadic coalescing function (takes the first non null).

We'd be allocating a new null bitmap on each call to when which is not ideal. However typing is far clearer (when(cond: Boolean, value: T): T, case(...values: T): T) and case/when can be independently unit tested.

@pitrou what do you think?

lidavidm · 2021-07-07T16:38:11Z

I've changed this so the signature is CaseWhen<T>(struct(bool...), T...) instead; if this looks more reasonable, we can proceed, else I'll revert it.

jorisvandenbossche · 2021-07-07T18:20:34Z

What's the advantage of using a struct of boolean arrays as first argument instead of a vector? (eg for a struct you need to give it (dummy) names)

lidavidm · 2021-07-07T18:25:20Z

@bkietz and I discussed this. Mostly it simplifies the function signature a lot and gets rid of the custom typechecking code in DispatchBest. It does require materializing scalar conditions and would need callers to do some pre-processing if they wish to avoid that (e.g. dropping any values associated with a scalar false/null condition, using values associated with a scalar true condition as the 'else' clause).

It's mostly a point of discussion. I realized I forgot to benchmark so let me do that now.

lidavidm · 2021-07-07T18:34:04Z

This is quite faster though (actually almost suspiciously so; also, the benchmark loop doesn't include constructing a structarray from individual boolean arrays).

Struct:

CaseWhenBench32/1048576/0               5780694 ns      5780134 ns          116 bytes_per_second=2.72434G/s
CaseWhenBench64/1048576/0               6079444 ns      6078645 ns          115 bytes_per_second=5.16103G/s
CaseWhenBench32/1048576/99              5717893 ns      5717306 ns          122 bytes_per_second=2.75402G/s
CaseWhenBench64/1048576/99              6160254 ns      6159488 ns          110 bytes_per_second=5.09281G/s
CaseWhenBench32Contiguous/1048576/0     1652127 ns      1645047 ns          423 bytes_per_second=7.19786G/s
CaseWhenBench64Contiguous/1048576/0     1986278 ns      1986066 ns          334 bytes_per_second=11.8624G/s
CaseWhenBench32Contiguous/1048576/99    1826875 ns      1826675 ns          377 bytes_per_second=6.48156G/s
CaseWhenBench64Contiguous/1048576/99    2415119 ns      2414697 ns          312 bytes_per_second=9.75582G/s

Alternating arguments: (EDIT: updated since I forgot to rebuild first)

CaseWhenBench32/1048576/0              32471264 ns     32470765 ns           22 bytes_per_second=496.601M/s
CaseWhenBench64/1048576/0              33443346 ns     33443152 ns           21 bytes_per_second=960.585M/s
CaseWhenBench32/1048576/99             33171599 ns     33171474 ns           21 bytes_per_second=486.065M/s
CaseWhenBench64/1048576/99             33970437 ns     33969940 ns           21 bytes_per_second=945.6M/s
CaseWhenBench32Contiguous/1048576/0     7543780 ns      7543809 ns           92 bytes_per_second=1.56961G/s
CaseWhenBench64Contiguous/1048576/0     7982627 ns      7982575 ns           86 bytes_per_second=2.95137G/s
CaseWhenBench32Contiguous/1048576/99    7894886 ns      7894844 ns           89 bytes_per_second=1.49968G/s
CaseWhenBench64Contiguous/1048576/99    8419204 ns      8419033 ns           82 bytes_per_second=2.79811G/s

lidavidm · 2021-07-13T15:15:50Z

Any other thoughts here? If we're ok with CaseWhen(struct(bool...), T...) over CaseWhen(bool, T, bool, T, ...) I can make this kernel also support writing into slices.

bkietz · 2021-07-13T15:17:49Z

I'm fine with CaseWhen(struct(bool...), T...), and I'd love for this to support writing into slices. Could you also file a follow up JIRA for simplification of call expressions involving case_when?

lidavidm · 2021-07-13T15:21:07Z

Filed ARROW-13325.

lidavidm · 2021-07-13T17:58:30Z

Updated to support can_write_into_slices. Also, updated the benchmark bytes_per_second scaling to reflect the size of the output and not the size of all the inputs so the numbers are more reasonable.

-----------------------------------------------------------------------------------------------
Benchmark                                     Time             CPU   Iterations UserCounters...
-----------------------------------------------------------------------------------------------
CaseWhenBench32/1048576/0               6065579 ns      6065602 ns           97 bytes_per_second=659.456M/s
CaseWhenBench64/1048576/0               6485780 ns      6485814 ns           93 bytes_per_second=1.20455G/s
CaseWhenBench32/1048576/99              6280547 ns      6280511 ns          114 bytes_per_second=636.831M/s
CaseWhenBench64/1048576/99              6694807 ns      6694710 ns          105 bytes_per_second=1.16686G/s
CaseWhenBench32Contiguous/1048576/0     1761802 ns      1761781 ns          378 bytes_per_second=2.21722G/s
CaseWhenBench64Contiguous/1048576/0     2267441 ns      2267379 ns          280 bytes_per_second=3.44561G/s
CaseWhenBench32Contiguous/1048576/99    1897469 ns      1897449 ns          325 bytes_per_second=2.05849G/s
CaseWhenBench64Contiguous/1048576/99    2515244 ns      2515231 ns          275 bytes_per_second=3.10578G/s

bkietz

Some minor comments, otherwise this looks good to go

bkietz · 2021-07-13T19:03:09Z

docs/source/cpp/compute.rst

This (and some of the other comments) need to be updated with the new signature

bkietz · 2021-07-13T19:07:36Z

cpp/src/arrow/compute/kernels/scalar_if_else_benchmark.cc

Suggested change

auto null_scalar = MakeNullScalar(boolean());

ASSERT_OK_AND_ASSIGN(auto nulls,

MakeArrayFromScalar(*null_scalar, len - 2 * (len / 3)));

ASSERT_OK_AND_ASSIGN(auto nulls, MakeArrayOfNull(boolean(), len - 2 * (len / 3)));

bkietz · 2021-07-13T19:13:37Z

cpp/src/arrow/compute/kernels/scalar_if_else_benchmark.cc

Suggested change

auto cond1 = std::static_pointer_cast<BooleanArray>(

rand.ArrayOf(boolean(), len, /*null_probability=*/0.01));

auto cond2 = std::static_pointer_cast<BooleanArray>(

rand.ArrayOf(boolean(), len, /*null_probability=*/0.01));

auto cond3 = std::static_pointer_cast<BooleanArray>(

rand.ArrayOf(boolean(), len, /*null_probability=*/0.01));

auto fld = field("cond", boolean(), key_value_metadata({{"null_probability", "1.0"}}));

auto cond = rand.ArrayOf(field("", struct_({fld, fld, fld}), len);

?

lidavidm · 2021-07-13T20:01:58Z

Fixed docstrings and fixed a TODO I noticed about fully initializing the output buffer.

Also added a benchmark case for when both the cond struct array and the child cond boolean arrays can have nulls. This case is especially terrible, I made a slight optimization to eliminate one of the more egregious offenders I saw in perf, but it's still bad even then:

-----------------------------------------------------------------------------------------------
Benchmark                                     Time             CPU   Iterations UserCounters...
-----------------------------------------------------------------------------------------------
CaseWhenBench64/1048576/0               6045379 ns      6045204 ns          125 bytes_per_second=1.29235G/s
CaseWhenBench64/1048576/99              6106510 ns      6106346 ns          123 bytes_per_second=1.27929G/s
CaseWhenBench64OuterNulls/1048576/0    32735686 ns     32733991 ns           23 bytes_per_second=244.394M/s
CaseWhenBench64OuterNulls/1048576/99   34098317 ns     34097473 ns           21 bytes_per_second=234.599M/s
CaseWhenBench64Contiguous/1048576/0     2229728 ns      2229687 ns          338 bytes_per_second=3.50386G/s
CaseWhenBench64Contiguous/1048576/99    2456084 ns      2456085 ns          237 bytes_per_second=3.18058G/s

bkietz · 2021-07-14T19:02:48Z

@lidavidm what would you think about just raising an error for top level nulls? It doesn't seem like a useful case to me

lidavidm · 2021-07-14T19:03:41Z

Should be fine. It would certainly trim down the inner loop a lot.

lidavidm · 2021-07-14T20:46:53Z

Removed support for toplevel nulls.

lidavidm · 2021-07-15T14:33:09Z

@bkietz anything left either here or in #10608?

jorisvandenbossche · 2021-07-15T15:36:06Z

Any other thoughts here? If we're ok with CaseWhen(struct(bool...), T...) over CaseWhen(bool, T, bool, T, ...)

As mentioned on the meeting, I don't have a strong opinion on the exact signature (assuming you can create the inputs easily in the bindings either way), and if it simplifies the implementation / signature quite a bit, that sounds as a good reason.

I quickly tried it out in Python:

In [7]: cond = pc.project(pa.array([True, False, None]), pa.array([False, True, None]), field_names=[b"a", b"b"])

In [8]: pc.case_when(cond, pa.array([1, 2, 3]), pa.array([11, 12, 13]))
Out[8]: 
<pyarrow.lib.Int64Array object at 0x7f104124c820>
[
  1,
  12,
  null
]

What I find a little bit annoying is that I have to provide "dummy" names (that are never used) to my boolean conditions in project (maybe we could have some default for this so for this specific case specifying names is not required?)
(I also noticed that the field names needed to be bytes and not strings, I suppose that's an error in the cython bindings, will open an issue about that).
One other issue is that this uses the "project" kernel, which I thought was not really meant for users? (ARROW-11206) I could of course also have used pa.StructArray.from_arrays I assume.

cpp/src/arrow/compute/kernels/scalar_if_else.cc

Co-authored-by: Joris Van den Bossche <[email protected]>

bkietz

LGTM.

@jorisvandenbossche I'll make a PR for ARROW-11206 shortly and allow make_struct to be called with no names provided (defaulting to empty names or str(field_index) or so). That should enable you to write

pc.case_when(
    pc.make_struct([True, False, None],
                   [False, True, None]),
    [1, 2, 3],
    [11, 12, 13])

bkietz · 2021-07-15T18:58:12Z

cpp/src/arrow/compute/kernel_test.cc

+    ASSERT_FALSE(sig.MatchesInputs(args));
+  }
+  {
+    KernelSignature sig({int8(), utf8()}, utf8(), /*is_varargs=*/true);


Thanks for working this out

jorisvandenbossche · 2021-07-16T05:45:45Z

@bkietz sounds good!

github-actions bot added the Component: C++ label Jun 18, 2021

lidavidm force-pushed the arrow-13064 branch 3 times, most recently from 3e7e6cb to a00643e Compare June 25, 2021 17:34

lidavidm mentioned this pull request Jun 25, 2021

ARROW-9430: [C++] Implement replace_with_mask kernel #10412

Closed

lidavidm force-pushed the arrow-13064 branch from a00643e to caf583f Compare June 28, 2021 13:25

lidavidm force-pushed the arrow-13064 branch from e268b2b to 40ac3cf Compare June 29, 2021 19:40

lidavidm force-pushed the arrow-13064 branch from 40ac3cf to e7106cd Compare June 30, 2021 15:01

pitrou requested changes Jun 30, 2021

View reviewed changes

bkietz requested changes Jun 30, 2021

View reviewed changes

lidavidm marked this pull request as draft July 1, 2021 17:14

lidavidm force-pushed the arrow-13064 branch from 375f6f1 to e533c79 Compare July 2, 2021 16:00

lidavidm marked this pull request as ready for review July 2, 2021 16:49

lidavidm force-pushed the arrow-13064 branch from 4fb8554 to e75a6ce Compare July 13, 2021 17:56

bkietz requested changes Jul 13, 2021

View reviewed changes

lidavidm force-pushed the arrow-13064 branch from e75a6ce to b0d2aa7 Compare July 13, 2021 20:00

lidavidm force-pushed the arrow-13064 branch from b0d2aa7 to 42dc5cc Compare July 14, 2021 15:48

lidavidm added 9 commits July 14, 2021 15:21

ARROW-13220: [C++] Implement 'case_when' for fixed-width types

a0a308e

ARROW-13064: [C++] Tweak signature of case_when

72dfd85

ARROW-13064: [C++] Update benchmark to only count output size

63c0034

ARROW-13064: [C++] Enable can_write_into_slices for case_when

ec49ff9

ARROW-13064: [C++] Update case_when docstrings

879da44

ARROW-13064: [C++] Benchmark another case

c64dd7b

ARROW-13064: [C++] Micro-optimize a particularly bad case

0087290

ARROW-13064: [C++] Make sure to initialize memory

0a8a3c7

ARROW-13064: [C++] Don't allow outer nulls

c4ea55f

lidavidm force-pushed the arrow-13064 branch from 42dc5cc to c4ea55f Compare July 14, 2021 19:21

jorisvandenbossche reviewed Jul 15, 2021

View reviewed changes

cpp/src/arrow/compute/kernels/scalar_if_else.cc Outdated Show resolved Hide resolved

lidavidm and others added 2 commits July 15, 2021 11:46

Update cpp/src/arrow/compute/kernels/scalar_if_else.cc

89287c6

Co-authored-by: Joris Van den Bossche <[email protected]>

ARROW-13064: [C++] Fix test

fd3458a

bkietz approved these changes Jul 15, 2021

View reviewed changes

bkietz closed this in dbeed52 Jul 15, 2021

asfimport mentioned this pull request Jul 16, 2021

[C++] Add a general "if, ifelse, ..., else" kernel ("CASE WHEN") #28773

Closed

-  auto null_scalar = MakeNullScalar(boolean());
-  ASSERT_OK_AND_ASSIGN(auto nulls,
-                       MakeArrayFromScalar(*null_scalar, len - 2 * (len / 3)));
+  ASSERT_OK_AND_ASSIGN(auto nulls, MakeArrayOfNull(boolean(), len - 2 * (len / 3)));

-  auto cond1 = std::static_pointer_cast<BooleanArray>(
-      rand.ArrayOf(boolean(), len, /*null_probability=*/0.01));
-  auto cond2 = std::static_pointer_cast<BooleanArray>(
-      rand.ArrayOf(boolean(), len, /*null_probability=*/0.01));
-  auto cond3 = std::static_pointer_cast<BooleanArray>(
-      rand.ArrayOf(boolean(), len, /*null_probability=*/0.01));
+  auto fld = field("cond", boolean(), key_value_metadata({{"null_probability", "1.0"}}));
+  auto cond = rand.ArrayOf(field("", struct_({fld, fld, fld}), len);

ARROW-13064: [C++] Implement select ('case when') function for fixed-width types #10557

ARROW-13064: [C++] Implement select ('case when') function for fixed-width types #10557

Uh oh!

Conversation

lidavidm commented Jun 18, 2021

Uh oh!

github-actions bot commented Jun 18, 2021

Uh oh!

lidavidm commented Jun 25, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jorisvandenbossche commented Jun 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lidavidm commented Jun 29, 2021

Uh oh!

pitrou commented Jun 29, 2021

Uh oh!

lidavidm commented Jun 29, 2021

Uh oh!

pitrou commented Jun 30, 2021

Uh oh!

lidavidm commented Jun 30, 2021

Uh oh!

pitrou left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

lidavidm commented Jun 25, 2021 •

edited

Loading

jorisvandenbossche commented Jun 29, 2021 •

edited

Loading

lidavidm commented Jul 7, 2021 •

edited

Loading