Feature/dask.array.apply_gufunc (Issue #702) by magonser · Pull Request #3109 · dask/dask

magonser · 2018-01-27T17:58:22Z

This is an attempt to solve #702. I think the PR is not quite finished yet:

Done:

Implement apply_gufunc and provide a variation gufunc
Tests added / passed
Passes flake8 dask
Fully documented, including docs/source/changelog.rst for all changes
and one of the docs/source/*-api.rst files for new API

Todo:

Possibly decide between apply_gufunc or gufunc
Maybe make extra doc-page for gufuncs.

Notes:

This implementation basically uses atop. I found that the following modifications could
improve this implementation:

Possible modifications to top or unify_chunks:
1. Make top such that specific dimensions could be passed to argument concatenate. This would be needed to make core-dims to have only one chunk, even for core-dims, which are present in the gufuncs output
2. Alternatively modify unify_chunks such, that required chunk amount can be specified. That way it also could be enforced that core-dims have one chunksize
atop with multiple outputs: Given how little code this implementation is, it should be possible to include multiple outputs also for atop. I've opted for this implementation, as it has been discussed in atop with multiple output arguments #702 and seems sensible
Implement something like broadcast_arrays as oulined in atop with multiple output arguments #702, that allows to explicitly broadcast array dimensions against each other by additional specifications. xarray.apply_ufunc implements such functionality already, but having this in dask could enhance usability independent of xarray.

mrocklin · 2018-01-27T18:03:23Z

I'm looking forward to reading through this (though probably not for several hours)

Also cc'ing @shoyer (atop) and @jakevdp (gufuncs)

shoyer · 2018-01-27T18:11:37Z

Awesome! I'm also looking forward to reading through this when I'm back home on Monday.

mrocklin · 2018-01-28T00:23:19Z

Possible modifications to top or unify_chunks:

Make top such that specific dimensions could be passed to argument concatenate. This would be needed to make core-dims to have only one chunk, even for core-dims, which are present in the gufuncs output

Alternatively modify unify_chunks such, that required chunk amount can be specified. That way it also could be enforced that core-dims have one chunksize

Perhaps we should call rechunk explicitly here? In some cases I can also imagine wanting to reduce the chunk sizes of the non-core dimensions in order to maintain a modest chunksize. This might be too much magic though, I'm not sure.

…lled

mrocklin

Some initital comments, mostly to help educate myself.

mrocklin · 2018-01-28T19:07:20Z

dask/array/gufunc.py

+    _outs = _parse_args(_out)
+    if _outs and (len(_outs) == 1) and ('),' not in _out):
+        _outs = _outs[0]
+    return _ins, _outs


It would ease reviewing and reading by future devs if these functions had relatively simple docstrings with a couple examples of their inputs/outputs

def parse_signature(signature): """" Parse gufunc signature into ... Examples -------- >>> parse_signature("...") ... """

mrocklin · 2018-01-28T19:08:52Z

dask/array/gufunc.py

+        set ``vectorize=True``).
+    signature: String
+        Specifies what core dimensions are consumed and produced by ``func``.
+        According to the specification of numpy.gufunc signature [2]_


Quick question, do numpy gufuncs have signatures attached to them? If so would it make sense to get the signature from the callable?

mrocklin · 2018-01-28T19:09:40Z

dask/array/gufunc.py

+    ...     return np.mean(x, axis=-1), np.std(x, axis=-1)
+    ... a = da.random.normal(size=(10,20,30), chunks=5)
+    ... mean, std = da.apply_gufunc(stats, "(i)->(),()", a, output_dtypes=2*(a.dtype,))
+    ... mean.compute()  # doctest: +SKIP


Style note, I recommend using >>> on each individual line.

Except the body of the stats function, of course

mrocklin · 2018-01-28T19:13:10Z

dask/array/gufunc.py

+    vectorize = kwargs.pop("vectorize", None)
+    concatenate = kwargs.pop("concatenate", True)
+    if output_dtypes is None:
+        raise ValueError("Must specify `output_dtypes` of output array(s)")


Typically we run the function on a small or empty array to see if that can produce the dtype for us. See dask/array/core.py::apply_infer_dtype

mrocklin · 2018-01-28T19:14:00Z

dask/array/gufunc.py

+        if len(output_dtypes) != nout:
+            raise ValueError("Must specify tuple of %d dtypes for `output_dtypes` for function with multiple outputs" % nout)
+    if nout is None and (isinstance(output_dtypes, tuple) or isinstance(output_dtypes, list)):
+        raise ValueError("Must specify single dtype for `output_dtypes` for function with one output")


Just checking, but do we test for this behavior? (I haven't gotten down to that yet)

mrocklin · 2018-01-28T19:18:10Z

dask/array/gufunc.py

+    tmp = atop(ufunc, loop_output_dims, *arginds,
+               dtype=int,  # Only dummy dtype, anyone will do
+               concatenate=concatenate,
+               **kwargs)


Given that we don't really want an array output, would it make sense to drop down to top here?

I'm also open to teaching atop how to handle multiple outputs. (I'll write a longer note about this in the main comments)

I also needed unify_chunks being called, so that I basically use 80% of atop code here. After below's discussion potential refactor can be continued.

mrocklin · 2018-01-28T19:19:43Z

dask/array/gufunc.py

+    return leaf_arrs if nout else leaf_arrs[0]
+
+
+@curry


Using curry can hide odd sorts of errors. If we know how this is likely to be used then it might be cleaner to make a function that returns another function explicitly.

I have removed it for now. Check for correctness of passed **kwargs is still postponed until apply_gufunc is called, i.e. very late. Depending on the other discussion about dtype inference, we can finalize the behavior here eventually

mrocklin · 2018-01-28T19:21:46Z

dask/array/gufunc.py

+    ...     return np.mean(x, axis=-1), np.std(x, axis=-1)
+    ... a = da.random.normal(size=(10,20,30), chunks=5)
+    ... mean, std = stats(a)
+    ... mean.compute()  # doctest: +SKIP


Ideally we would define this thing as a numpy gufunc and implement some protocol in dask to handle those when applied to dask arrays. I suspect that this isn't doable today. I'll write more about this in a longer comment.

mrocklin · 2018-01-28T19:24:36Z

Regarding atop with multiple arguments I'm all in favor of doing this if it's easy to do and doesn't impact the easy growth of atop in the future. My guess is that this will be harder to do in general due to all of the other features that have been added to atop in the past. As we add features to atop I think we need to be careful that they are added in such a way that is nicely orthogonal and respectful of the needs of future changes.

mrocklin · 2018-01-28T19:30:15Z

The combination of gufuncs and dask.array is a powerful one. In my ideal world we wouldn't define these as da.gufuncs as we've done in this example ...

@dask.array.gufunc("(i)->(),()", output_dtypes=2*(a.dtype,))
def stats(x):
    return np.mean(x, axis=-1), np.std(x, axis=-1)

... but rather as numpy.gufuncs.

@numpy.gufunc("(i)->(),()", output_dtypes=2*(a.dtype,))
def stats(x):
    return np.mean(x, axis=-1), np.std(x, axis=-1)

Then we would implement some sort of __array_gufunc__ protocol and call your apply_gufunc operation whenever a gufunc was called on a set of arguments that included a dask array. CC @njsmith . My guess is that this isn't doable today, but is in scope for medium-term enhancements. Is this guess correct?

This would also be a nice touchpoint for other projects that are able to create gufuncs, like numba. cc @seibert

shoyer · 2018-01-28T19:43:55Z

It should be totally doable today to make Dask array directly support NumPy gufuncs. Gufuncs call __array_ufunc__ and expose their signature with the signature attribute (if I remember correctly).

magonser · 2018-01-29T19:29:36Z

Thanks for your comments.

Perhaps we should call rechunk explicitly here? In some cases I can also imagine wanting to reduce the chunk sizes of the non-core dimensions in order to maintain a modest chunksize. This might be too much magic though, I'm not sure.

In it's current implementation, apply_gufunc relies on unify_chunks, which is called by atop for rechunking. Maybe there are too many different use cases to find good rules about how to auto-magically rechunk?

It should be totally doable today to make Dask array directly support NumPy gufuncs. Gufuncs call array_ufunc and expose their signature with the signature attribute (if I remember correctly).

That means, we could automatically map numpy gufuncs. But how about plain python functions, which shall be used as gufunc as shown in this PRs use cases and examples. I.e. in numpy either you don't wrap your function at all, because you expect numpy arrays and use only numpy math, or if you want to vectorize it, use numpy.vectorize which supports the gufunc signature. Now, for dask there is an added layer in between of course - blocks. So I want to wrap my function like numpy.vectorize, but expect the blocks being paassed as numpy arrays including the loopdims for performance reasons. Maybe the chosen name apply_gufunc/gufunc is misleading here - any other suggestions?

Extending atop for many outputs is handy in itself, but I consider keeping function's signature (and the core-dimensions) separate from an added preliminary broadcast stage for the data at hand, a sensible thing to do.

shoyer

A few comments on the signature parsing part of the code.

shoyer · 2018-01-29T19:23:37Z

dask/array/gufunc.py

+    return list(r for r in ret if r is not None)
+
+
+def parse_signature(signature):


You might just borrow the version of this function that I wrote for numpy.vectorize:
https://github.com/numpy/numpy/blob/99019107cbde06294a356fd7fc859f9b31f87410/numpy/lib/function_base.py#L1638-L1666

shoyer · 2018-01-29T19:24:41Z

dask/array/gufunc.py

+    >>> parse_signature('(i,j)->(i),(j)')
+    ([('i', 'j')], [('i',), ('j',)])
+
+    >>> parse_signature('->(i),')


I don't think this should be a valid gufunc signature. The spec doesn't allow trailing commas.

shoyer · 2018-01-29T19:25:20Z

dask/array/gufunc.py

+    _ins = [i for i in _ins if i is not None]
+    _outs = _parse_args(_out)
+    if _outs and (len(_outs) == 1) and ('),' not in _out):
+        _outs = _outs[0]


These sort of special cases (return a scalar for length 1 output) are usually best avoided. They end up requiring work arounds in code that uses it.

Thank you for pointing out. It confuses me though: Return arguments in Python can be either a scalar or tuples, even if only length 1. Python function return statements handle this consistently, i.e.

def foo(): return 1, a, = foo()

I don’t understand, why numpy.gufunc signature forbids length 1 return tuples. It doesn’t limit the normal usage of returning scalars.

Shouldn't functions be implemented that they behave consistently even for corner cases? E.g. also the signature "->()" or "->(i)" are currently not valid gufunc signatures. While it seems funny to wrap a non-gufunc without any arguments as gufunc, it should lead to a consistent workflow in the end.

I understand that for now, this should be implemented according to current numpy definition of the signature.

What is your stand on this? Should the signature definition in numpy be extended to cover mentioned corner cases?

It might be worth bringing this up as a NumPy issue

shoyer · 2018-01-29T19:27:47Z

dask/array/gufunc.py

+        _flag = len(outs) == 1
+        outs = ",".join(("({0})".format(",".join(dims)) for dims in outs))
+        if _flag:
+            outs += ","


I'm not quite sure why you need this?

shoyer · 2018-01-29T19:28:57Z

dask/array/gufunc.py

+        outs = ",".join(("({0})".format(",".join(dims)) for dims in outs))
+        if _flag:
+            outs += ","
+    else:


I think this clause could be dropped if you only support the versions always takes multiple outputs.

shoyer · 2018-01-29T19:39:33Z

That means, we could automatically map numpy gufuncs. But how about plain python functions, which shall be used as gufunc as shown in this PRs use cases and examples.

I agree, handling non-gufuncs that work like gufuncs is an important use case. It's a pain to write actual gufuncs (they need to be in C and have limited features), so even NumPy has some non-gufunc gufunc-likes like numpy.matmul, np.linalg.inv and np.vectorize.

mrocklin · 2018-01-29T20:16:19Z

That means, we could automatically map numpy gufuncs. But how about plain python functions, which shall be used as gufunc as shown in this PRs use cases and examples.

I agree, handling non-gufuncs that work like gufuncs is an important use case. It's a pain to write actual gufuncs (they need to be in C and have limited features), so even NumPy has some non-gufunc gufunc-likes like numpy.matmul, np.linalg.inv and np.vectorize.

I also agree. It would be good to coordinate with Numpy devs so that we come to an interface that they might also be happy with in the future. It would be nice at some point to implement this as follows:

if numpy.__version__ >= ...:
    gufunc = numpy.gufunc
else:
    def gufunc(...)

mrocklin · 2018-02-02T20:25:29Z

Checking on the status here. This has been dormant for a few days. Is there anything that's blocking you @magonser ? No time pressure of course, I just want to make sure that you're able to move smoothly if/when you have time.

magonser · 2018-02-05T19:10:28Z

Alive beat! Sorry for not getting back fast. Two reasons: this is in my spare time (so either way, this will take some time) and I am still wrapping my head around the necessary actions, and numpy internals.

My current understanding is

numpy:

Raise issue #10526 for general additional method gufunc for wrapping pyfuncs into gufunc objects, just like numpy.vectorize, but without the "vectorize" part (broadcast of loop dimensions would take place)
Raise issue #10527 for extending corner-cases of signature definition

dask (this PR):

Make signature consistent with current implementation of np.vectorize
Hook gufunc inside dask.array.array.__array_ufunc__ routine
Provide own gufunc wrapper until numpy might provide it's own implementation, as suggested by @mrocklin

mrocklin · 2018-02-22T15:50:47Z

Checking in again on this. No time pressure, I just want to track things. Are you blocking on anything from other Dask maintainers? Numpy maintainers?

magonser · 2018-02-27T19:30:41Z

With respect to the above mentioned two numpy issues there haven’t been too many discussions. Especially aligning API has not received any comments. I don’t think it’s blocking anything, but I’ll just be needing more time. Is it better for you to reopen this PR later?

mrocklin · 2018-05-16T15:35:14Z

It looks like there are some failures, perhaps based on numpy versions:

�[1m        if vectorize:�[0m
�[1m>           func = np.vectorize(func, signature=signature, otypes=otypes)�[0m
�[1m�[31mE           TypeError: __init__() got an unexpected keyword argument 'signature'�[0m

…ize(..., signature=...)` support

mrocklin · 2018-05-16T18:00:07Z

@shoyer any further comments here?

magonser · 2018-05-16T18:08:19Z

None from my side. Let me know, if I need to change more things.

At this point: I thank you for your patience with this PR and your time for the review and discussions - I appreciate it.

mrocklin · 2018-05-16T21:23:04Z

I'm generally pretty happy with this. Merging tomorrow morning US Eastern time if there are no further comments.

mrocklin · 2018-05-16T21:24:49Z

@teoliphant I suspect that you might enjoy this work.

https://github.com/magonser/dask/blob/f97605eecf76d0cf8ea0473d2fc7b3a8321e259b/docs/source/array-gufunc.rst

shoyer · 2018-05-16T21:25:11Z

dask/array/gufunc.py

+    # Input processing:
+    ## Signature
+    if not isinstance(signature, str):
+        raise ValueError('`signature` has to be of type string')


nit: this should be TypeError

shoyer · 2018-05-16T21:28:11Z

dask/array/gufunc.py

+           for arg in re.findall(_ARGUMENT, in_txt)]
+    outs = [tuple(re.findall(_DIMENSION_NAME, arg))
+            for arg in re.findall(_ARGUMENT, out_txt)]
+    outs = outs[0] if ((len(outs) == 1) and (out_txt[-1] != ',')) else outs


Why does this have a different return type for a single output? I think that makes your logic below unnecessarily complicated.

shoyer · 2018-05-16T21:29:27Z

dask/array/gufunc.py

+    core_input_dimss, core_output_dimss = _parse_gufunc_signature(signature)
+
+    ## Determine nout: nout = None for functions of one direct return; nout = int for return tuples
+    nout = None if not isinstance(core_output_dimss, list) else len(core_output_dimss)


Why use nout=None instead of the simpler nout=1?

shoyer · 2018-05-16T21:30:56Z

dask/array/gufunc.py

+    else:
+        if nout is not None:
+            raise ValueError("Must specify tuple of dtypes for `output_dtypes` for function with multiple outputs")
+        otypes = [output_dtypes]


I don't quite understand why all this logic is necessary. Are you trying to be more flexible in allowing a single output_dtype to be provided if there is only one output?

shoyer · 2018-05-16T21:31:40Z

dask/array/gufunc.py

+    ### *) Treat direct output
+    if nout is None:
+        core_output_dimss = [core_output_dimss]
+        output_dtypes = [output_dtypes]


If you avoided unpacking core_output_dimss and output_dtypes above, I don't think you would need this step.

shoyer · 2018-05-16T21:33:44Z

dask/array/tests/test_array_core.py

+def test_Array_numpy_gufunc_call__array_ufunc__01():
+    x = da.random.normal(size=(3, 10, 10), chunks=(2, 10, 10))
+    nx = x.compute()
+    ny = np.linalg._umath_linalg.inv(nx)


Can you use the public np.linalg.inv with apply_gufunc? It's not a good idea to rely on functions in private modules, because those could change in any release without warning.

I guess we could stick with this (since you document these methods in the docs, and numpy doesn't expose any gufuncs in its public API currently), but it makes me a little nervous. Maybe it would be better to make the skipif condition hasattr(hasattr(np.linalg, '_umath_linalg'), 'inv')`.

Will do tomorrow

Make _umath_linalg tests optional

The problem seems, that the private functions exist already, but do not receive optional kwargs

Using numpy=1.11:

____________________________________ test_Array_numpy_gufunc_call__array_ufunc__01 _____________________________________ @pytest.mark.skipif(hasattr(hasattr(np.linalg, '_umath_linalg'), 'inv'), reason="NumPy doesn't have `np.linalg._umath_linalg.inv`") def test_Array_numpy_gufunc_call__array_ufunc__01(): x = da.random.normal(size=(3, 10, 10), chunks=(2, 10, 10)) nx = x.compute() ny = np.linalg._umath_linalg.inv(nx) > y = np.linalg._umath_linalg.inv(x, output_dtypes=float) E TypeError: 'output_dtypes' is an invalid keyword to ufunc 'inv' dask/array/tests/test_array_core.py:226: TypeError =============================================== 1 failed in 0.76 seconds ===============================================

I've added an pytest.mark.xfail to both relevant tests

shoyer · 2018-05-16T21:39:32Z

dask/array/tests/test_gufunc.py

+# def test_apply_gufunc_one_scalar_output():
+#     def foo():
+#         return 1,
+#     x, = apply_gufunc(foo, "->(),", output_dtypes=(int,))


OK, now I understand why you special cased one output. Remind me, did you open an upstream issue in NumPy to discuss this?

My guess is that even though this is arguably poor API (special casing one vs. many return values) there will be little appetite for changing/extending it in NumPy itself.

Yes, the upstream issue is numpy/numpy#10527

I slowly get to understand that I might need to convince many people for this to change upstream and that the order should be to implement against current API and change later if upstream changes.

Of course all your other code comments vanish, if I'd roll back to the current signature API.

As mentioned before I'd like to be transparent about the current implementation, which means also (focus on output_dtypes):

def foo(x): return x a = np.array([1, 2, 3]) apply_gufunc(foo, "()->()", a, output_dtypes=int) # Not: output_dtypes=[int]

What do you recommend/favor? Keep or roll back to current signature API?

I do think it would be slightly better not to try to extend the gufunc signature here (before we have concensus that it makes sense for numpy). There aren't a lot of use-cases for gufuncs returning a tuple of length 1 (beyond consistency) or for accepting no input arguments, and consistency with numpy is virtuous.

That said, given that you already wrote this, I don't think there's a lot of harm in leaving it in as unsupported features...

Thanks @shoyer.

Ok - so we will keep these parts as they are now.

…th_linalg` methods

This reverts commit d2b27bf.

mrocklin · 2018-05-18T11:51:18Z

Checking in. What is the status here?

Any further concerns @shoyer? If not then would you like to do the honors and merge?

shoyer · 2018-05-18T18:35:29Z

thanks @magonser!

I'm really excited about this.

mrocklin · 2018-05-18T18:36:08Z

Huzzah!

…

On Fri, May 18, 2018 at 2:35 PM, Stephan Hoyer ***@***.***> wrote: thanks @magonser <https://github.com/magonser>! I'm really excited about this. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3109 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszM2CT8NUb5LlMe7PkFVSxTJFbjpUks5tzxRygaJpZM4RvYaX> .

mrocklin · 2018-05-18T18:36:48Z

Thank you for doing this @magonser and sticking with it. I'm very happy about this change

magonser · 2018-05-18T19:29:32Z

Awesome! Thanks also again for your patience!

Let's see what will come about it, once people start experimenting with it...

mrocklin · 2018-05-18T19:30:04Z

Already done: numba/numba#2979

…

On Fri, May 18, 2018 at 3:29 PM, Markus Gonser ***@***.***> wrote: Awesome! Thanks also again for your patience! Let's see what will come about it, once people start experimenting with it... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3109 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszEya-e24E5iEMjYZ59zf1922EciOks5tzyEdgaJpZM4RvYaX> .

jakirkham · 2018-05-19T03:14:05Z

Thanks for staying with the @magonser. Appreciate that pursuing a change like this is quite hard and it's really great that you have stuck with this and we were able to get in. Looking forward to playing with this in the new Dask version.

magonser added 7 commits January 27, 2018 15:40

Initial (still buggy) implementation of apply_gufunc

16554ad

Finalize initial bugfree implementation

03a7fde

Wrapup tasks

efee8e7

Tiny out of topic linebreak fix for docu

f38908c

Add new methods to docu

cd95613

Minor

3762bcc

Minor

e0e1aca

Minor docu fix

a92fa23

magonser added 2 commits January 28, 2018 11:08

Stylefixes, modify doc examples

2f12abb

Fix conceptual issue: loop dims get broadcast fully before func is ca…

fc724d5

…lled

mrocklin reviewed Jan 28, 2018

View reviewed changes

magonser added 3 commits January 29, 2018 20:13

Add docstring

1591653

Remove curry from gufunc

6c6707d

Stylefixes

95d9775

shoyer reviewed Jan 29, 2018

View reviewed changes

This was referenced Feb 5, 2018

Implement gufunc(pyfunc, ...) similar to numpy.vectorize but without the vectorize part numpy/numpy#10526

Open

Expand definition of generalized ufunc signature for corner cases numpy/numpy#10527

Open

mrocklin mentioned this pull request May 16, 2018

Automatic rechunking along particular axes #3506

Closed

Make some tests optional for numpy<1.12 because of missing `np.vector…

9fa16df

…ize(..., signature=...)` support

[skip ci] fix api link

f97605e

shoyer reviewed May 16, 2018

View reviewed changes

magonser added 5 commits May 17, 2018 20:04

Minor typo

1514793

Correct input validation to TypeError

d2b27bf

Add pytest.mark.xfail to tests that call private `numpy.linalg._uma…

846ffe7

…th_linalg` methods

Revert "Correct input validation to TypeError"

4152e64

This reverts commit d2b27bf.

Correct input validation to TypeError

ea6c2cb

shoyer merged commit bf5cfb0 into dask:master May 18, 2018

magonser deleted the feature/dask.array.apply_gufunc branch May 19, 2018 08:53

This was referenced May 23, 2018

Fixes bug, where size of dimensions would not always be checked #3526

Merged

Proposing dask.array.map_blocks function with multiple outputs #1887

Closed

jakirkham mentioned this pull request Aug 2, 2018

Possible to use Generalized Ufuncs? #1176

Closed

magonser mentioned this pull request Sep 4, 2018

Apply gufunc infer dtype #3936

Merged

2 tasks

datajanko mentioned this pull request Sep 21, 2018

[MRG] Poly trans: Issue #347 dask/dask-ml#367

Merged

jhamman mentioned this pull request Nov 19, 2018

apply_ufunc(dask='parallelized') with multiple outputs pydata/xarray#1815

Closed

jrbourbeau mentioned this pull request Aug 13, 2019

Set strict xfail option #5220

Merged

2 tasks

		return list(r for r in ret if r is not None)


		def parse_signature(signature):

Uh oh!

Conversation

magonser commented Jan 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Done:

Todo:

Notes:

Uh oh!

mrocklin commented Jan 27, 2018

Uh oh!

shoyer commented Jan 27, 2018

Uh oh!

mrocklin commented Jan 28, 2018

Uh oh!

mrocklin 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!

mrocklin commented Jan 28, 2018

Uh oh!

mrocklin commented Jan 28, 2018

Uh oh!

shoyer commented Jan 28, 2018

Uh oh!

magonser commented Jan 29, 2018

Uh oh!

shoyer 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!

shoyer commented Jan 29, 2018

Uh oh!

mrocklin commented Jan 29, 2018

Uh oh!

mrocklin commented Feb 2, 2018

Uh oh!

magonser commented Feb 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrocklin commented Feb 22, 2018

Uh oh!

magonser commented Feb 27, 2018

magonser commented Jan 27, 2018 •

edited

Loading

magonser commented Feb 5, 2018 •

edited

Loading

magonser May 16, 2018 •

edited

Loading