MAINT: wrap sparsetools manually instead via SWIG #3440

pv · 2014-03-08T19:19:54Z

This PR changes sparsetools to be wrapped via custom code generation rather than via SWIG.

This reduces memory and time requirements for compiling sparsetools. The approach is scalable: the set of routines can be easily split into smaller compilation units if necessary. Code generation is done at build time, so there are also no huge autogenerated files checked in the VCS.

Reduction in memory requirements likely originates from tighter, manually written, Python argument parsing code, which is shared between all routines.

A convenience drawback is that the argument specs for each function needs to be typed in manually, rather than parsed directly from the C++ code. However, this is implemented so that errors in the argument list specs result to compilation errors rather than crashes at runtime.

I also took a look at Cython: fused types don't seem to play together with C++ templates at the moment. A similar code generation framework and call_thunk would be then needed also with Cython, and I think the present approach is less magical. (The other suggestion of replacing this with templated C also requires the type dispatch mechanism to be written, and would probably cause pain when rewriting the boolean and complex ops.)

Some statistics:

#
# Before (master @ 7c64f1969)
#
$ /usr/bin/time -v g++ -Wall -D__STDC_FORMAT_MACROS -O3 -c -o /dev/null -Isparsetools -I/usr/include/python2.7 sparsetools/bsr_wrap.cxx
    Elapsed (wall clock) time (h:mm:ss or m:ss): 1:49.53
    Maximum resident set size (kbytes): 1140424

$ /usr/bin/time -v g++ -Wall -D__STDC_FORMAT_MACROS -O3 -c -o /dev/null -Isparsetools -I/usr/include/python2.7 sparsetools/csr_wrap.cxx
    Elapsed (wall clock) time (h:mm:ss or m:ss): 1:14.86
    Maximum resident set size (kbytes): 1089156

$ sloccount sparsetools (counting autogenerated files, too)
Total Physical Source Lines of Code (SLOC)                = 440,250

#
# After
#
$ /usr/bin/time -v g++ -Wall -D__STDC_FORMAT_MACROS -O3 -c -o /dev/null -Isparsetools -I/usr/include/python2.7 sparsetools/bsr.cxx 
    Elapsed (wall clock) time (h:mm:ss or m:ss): 1:12.27
    Maximum resident set size (kbytes): 672080

$ /usr/bin/time -v g++ -Wall -D__STDC_FORMAT_MACROS -O3 -c -o /dev/null -Isparsetools -I/usr/include/python2.7 sparsetools/csr.cxx 
    Elapsed (wall clock) time (h:mm:ss or m:ss): 0:29.09
    Maximum resident set size (kbytes): 469900

$ sloccount sparsetools (counting autogenerated files, too)
Total Physical Source Lines of Code (SLOC)                = 10,302

coveralls · 2014-03-08T21:20:32Z

Coverage remained the same when pulling be4d498 on pv:c-sparsetools into 7c64f19 on scipy:master.

rgommers · 2014-03-09T10:58:55Z

Awesome work! Works for me on 32-bit linux, now testing the release builds.

rgommers · 2014-03-09T11:01:11Z

Other observations besides compile time and memory usage:

test suite runs ~2% faster for me (consistent for several retries).
we now get one 29 Mb .so out instead of six .so's which together are ~20 Mb.

rgommers · 2014-03-09T11:23:38Z

Doesn't work on OS X yet:

relevant part of build log for 10.9: https://gist.github.com/rgommers/9446293
and for 10.6: https://gist.github.com/rgommers/9446340

pv · 2014-03-09T14:12:09Z

The OSX issue seemed to be some header ordering problem (Python.h must be included before C++ stdlib headers, for whatever reason). Fixed.

The .so sizes after stripping debug symbols are for me: before 9.9 MB, after 3.3MB. (Which reflects the SLOC reductions.)

coveralls · 2014-03-09T14:56:21Z

Coverage remained the same when pulling e3e6b09 on pv:c-sparsetools into 7c64f19 on scipy:master.

rgommers · 2014-03-09T15:48:05Z

Builds on both OS X versions now, so good to go and backport to 0.14.x I think. Also tested with scikit-learn, works fine.

rgommers · 2014-03-09T19:01:26Z

Fixes gh-2944 by removing all the warnings generated by Clang due to SWIG.

rgommers · 2014-03-09T19:02:07Z

And will stop code analysis tools/sites from reporting that Scipy is a C++ project:)

rgommers · 2014-03-09T20:09:26Z

Minor issue, this doesn't seem to work on Windows with MinGW:

Many hundreds of these:

======================================================================
ERROR: test_base.TestDOK.test_asfptype
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\nose-1.1.2-py3.2.egg\nose\case.py", line 198, in runTest
    self.test(*self.arg)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\tests\test_base.py", line 944, in test_asfptype
    assert_equal(A.asfptype().dtype, np.dtype('float64'))
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\base.py", line 114, in asfptype
    return self.astype(fp_type)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\base.py", line 102, in astype
    return self.tocsr().astype(t).asformat(self.format)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\dok.py", line 467, in tocsr
    return self.tocoo().tocsr()
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\coo.py", line 360, in tocsr
    A.sum_duplicates()
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\compressed.py", line 952, in sum_duplicates
    if self.has_canonical_format:
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\base.py", line 499, in __getattr__
    raise AttributeError(attr + " not found")
AttributeError: has_canonical_format not found

Quite a few of these:

======================================================================
ERROR: test_base.Test64Bit.test_resiliency_random(<class 'test_base.TestLIL'>, 'test_iterator')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\nose-1.1.2-py3.2.egg\nose\case.py", line 198, in runTest
    self.test(*self.arg)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\numpy\testing\decorators.py", line 147, in skipper_func
    return f(*args, **kwargs)
  File "<string>", line 2, in check
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\tests\test_base.py", line 105, in deco
    return func(*a, **kw)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\tests\test_base.py", line 3890, in check
    instance = cls()
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\tests\test_base.py", line 157, in __init__
    self.datsp = self.spmatrix(self.dat)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\lil.py", line 125, in __init__
    A = csr_matrix(A, dtype=dtype).tolil()
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\compressed.py", line 68, in __init__
    self._set_self(self.__class__(coo_matrix(arg1, dtype=dtype)))
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\compressed.py", line 31, in __init__
    arg1 = arg1.asformat(self.format)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\base.py", line 213, in asformat
    return getattr(self,'to' + format)()
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\coo.py", line 357, in tocsr
    data)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\sparsetools\coo.py", line 104, in coo_tocsr
    return _coo.coo_tocsr(*args)
NotImplementedError: Wrong number or type of arguments for overloaded function 'coo_tocsr'.
  Possible C/C++ prototypes are:
    coo_tocsr< int,npy_bool_wrapper >(int const,int const,int const,int const [],int const [],npy_bool_wrapper const [],int [],int [],npy_bool_wrapper [])
    coo_tocsr< int,signed char >(int const,int const,int const,int const [],int const [],signed char const [],int [],int [],signed char [])
    coo_tocsr< int,unsigned char >(int const,int const,int const,int const [],int const [],unsigned char const [],int [],int [],unsigned char [])
    coo_tocsr< int,short >(int const,int const,int const,int const [],int const [],short const [],int [],int [],short [])
    coo_tocsr< int,unsigned short >(int const,int const,int const,int const [],int const [],unsigned short const [],int [],int [],unsigned short [])
    coo_tocsr< int,int >(int const,int const,int const,int const [],int const [],int const [],int [],int [],int [])
    coo_tocsr< int,unsigned int >(int const,int const,int const,int const [],int const [],unsigned int const [],int [],int [],unsigned int [])
    coo_tocsr< int,long long >(int const,int const,int const,int const [],int const [],long long const [],int [],int [],long long [])
    coo_tocsr< int,unsigned long long >(int const,int const,int const,int const [],int const [],unsigned long long const [],int [],int [],unsigned long long [])
    coo_tocsr< int,float >(int const,int const,int const,int const [],int const [],float const [],int [],int [],float [])
    coo_tocsr< int,double >(int const,int const,int const,int const [],int const [],double const [],int [],int [],double [])
    coo_tocsr< int,long double >(int const,int const,int const,int const [],int const [],long double const [],int [],int [],long double [])
    coo_tocsr< int,npy_cfloat_wrapper >(int const,int const,int const,int const [],int const [],npy_cfloat_wrapper const [],int [],int [],npy_cfloat_wrapper [])
    coo_tocsr< int,npy_cdouble_wrapper >(int const,int const,int const,int const [],int const [],npy_cdouble_wrapper const [],int [],int [],npy_cdouble_wrapper [])
    coo_tocsr< int,npy_clongdouble_wrapper >(int const,int const,int const,int const [],int const [],npy_clongdouble_wrapper const [],int [],int [],npy_clongdouble_wrapper [])

And this one just once:

======================================================================
FAIL: test_base.TestCOONonCanonical.test_astype
----------------------------------------------------------------------
Traceback (most recent call last):
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\nose-1.1.2-py3.2.egg\nose\case.py", line 198, in runTest
    self.test(*self.arg)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\scipy\sparse\tests\test_base.py", line 937, in test_astype
    assert_equal(S.astype(x).toarray(), D.astype(x))        # correct values
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\numpy\testing\utils.py", line 256, in assert_equal
    return assert_array_equal(actual, desired, err_msg, verbose)
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\numpy\testing\utils.py", line 686, in assert_array_equal
    verbose=verbose, header='Arrays are not equal')
  File "Z:\Users\rgommers\.wine\drive_c\Python32\lib\site-packages\numpy\testing\utils.py", line 618, in assert_array_compare
    raise AssertionError(msg)
AssertionError: 
Arrays are not equal

(mismatch 22.22222222222223%)
 x: array([[4294967297,          0,          0],
       [         0, 4294967303,          0],
       [         0,          0,          0]], dtype=uint64)
 y: array([[1, 0, 0],
       [0, 7, 0],
       [0, 0, 0]], dtype=uint64)

pv · 2014-03-09T20:33:44Z

Maybe you have some left-over files from previous build? "Possible C/C++ prototypes are:" is a message from SWIG.

rgommers · 2014-03-09T20:48:41Z

That's possible, I didn't manually clean out site-packages. I'll check why they are picked up, probably that can be prevented by some renaming. Otherwise a lot of people could run into this on upgrade.

rgommers · 2014-03-09T21:05:10Z

The test_base.TestCOONonCanonical.test_astype failure is real. There's four of those, for COO, BSR, CSC and CSR. The rest is indeed due to old files left behind.

rgommers · 2014-03-09T21:15:55Z

The issue was that there's now a sparsetools.pyd and also an older installed submodule sparsetools/. The latter is picked up first. Can you rename the new extension _sparsetools?

… better

Installing over a previous Scipy installation causes problems, as sparsetools is not a package any more. Therefore, change the name. At the same time, mark the module for internal use only.

pv · 2014-03-12T20:11:33Z

done, and rebased

pv · 2014-03-12T20:30:13Z

Not sure what's up with the noncanonical astype failure. Does not occur on 32-bit linux.

rgommers · 2014-03-12T20:44:18Z

Shall we merge this and figure that last one out after the beta? Would be good to get that out the door finally.

pv · 2014-03-12T21:02:44Z

Would be better to resolve it first, as it probably indicates something is wrong.

rgommers · 2014-03-12T21:13:44Z

OK. Probably won't be able to look at that before Saturday though.

rgommers · 2014-03-15T20:05:43Z

It looks like the test is wrong. I have a bit of a hard time figuring out why the noncanonical tests are written the way they are, but what's going on is that this code:

def _same_sum_duplicate(data, *inds, **kwargs):
    """Duplicates entries to produce the same matrix"""
    ...
    data = data.repeat(2, axis=0)
    data[::2] *= 2
    data[1::2] *= -1

returns complex arrays with negative numbers for the failing test and those turn into large positions ones after astype(uint64) (also on Linux). Sparse matrices shouldn't have duplicate coordinates, so why is this tested this way? On Linux the toarray call happens to pick the positive complex values while under Wine it picks the negative ones.

If I insert the following debug prints the output should show what I described above:

class _NonCanonicalMixin(object):
    def spmatrix(self, D, **kwargs):
        """Replace D with a non-canonical equivalent"""
        construct = super(_NonCanonicalMixin, self).spmatrix
        M = construct(D, **kwargs)
        arg1 = self._arg1_for_noncanonical(M)
        if 'shape' not in kwargs:
            kwargs['shape'] = M.shape
        NC = construct(arg1, **kwargs)
        print('NC:  ', NC)
        print('NC type:  ', type(NC))
        print('NC astype:  ', NC.astype(np.uint64))
        print('NC astype.toarray:  ', NC.astype(np.uint64).toarray())
        assert_allclose(M.A, NC.A)
        return NC

Output for one failing test (the COO one) on Linux:

NC:     (0, 0)  (2+6j)
  (0, 0)        (-1-3j)
  (1, 1)        (14+0j)
  (1, 1)        (-7+0j)
NC type:   <class 'scipy.sparse.coo.coo_matrix'>
NC astype:     (0, 0)   2
  (0, 0)        18446744073709551615
  (1, 1)        14
  (1, 1)        18446744073709551609
NC astype.toarray:   [[1 0 0]
 [0 7 0]
 [0 0 0]]

And under Wine:

NC:     (0, 0)  (2+6j)
  (0, 0)        (-1-3j)
  (1, 1)        (14+0j)
  (1, 1)        (-7+0j)
NC type:   <class 'scipy.sparse.coo.coo_matrix'>
NC astype:     (0, 0)   2
  (0, 0)        4294967297
  (1, 1)        14
  (1, 1)        4294967303
NC astype.toarray:   [[4294967297        0        0]
 [       0 4294967303        0]
 [       0        0        0]]

pv · 2014-03-15T20:26:52Z

Ok, in that case it's probably safe to just merge, and mark the test as knownfailure, similarly as several other noncanonical tests.

coo and csr/csc matrices can in principle be allowed to contain duplicate entries, as this is convenient for construction.
Some of the routines currently treat such inputs incorrectly, so @jnothman wrote some test pre-emptively to track the status.
It would be useful to check why the test is not testing what it should, but this seems to be unrelated to the de-swigification.

rgommers · 2014-03-15T20:44:19Z

OK I'll do that and add some comment about the failure pointing to this discussion. Thanks.

jnothman · 2014-03-15T21:13:53Z

Sparse matrices shouldn't have duplicate coordinates

The COO matrix documentation makes explicit that duplicates are permitted, with the claim that "Duplicate (i,j) entries are summed when converting to CSR or CSC."
Yet their public sum_duplicates method, and a few code paths specifically to handle the duplicate cases in CSR/CSC, it seemed like we should actually ensure they're working (and they were some times).

Clearly astype won't work safely in the duplicate case for certain casts, unless duplicates are first checked for and summed.

rgommers · 2014-03-15T21:52:23Z

Ah, missed that in the docs. It's a bit incomplete, they should always be summed (like in toarray and not only during conversion to CSR/CSC).

Regarding the failures: I don't see what summing duplicates has to do with astype being safe for casts, either way complex to uint is never safe. The problem here is not astype, it's the test.

MAINT: wrap sparsetools manually instead via SWIG

jnothman · 2014-03-15T22:58:17Z

The problem here is not astype, it's the test.

I haven't understood where astype is being called here anyway. The test ensures duplicates are treated properly by artificially creating duplicates: for a matrix with entries x, concatenate -x and 2*x. This should be an effective test for any signed type, as far as I can tell.

Issue was that negative numbers were produced that were then going through astype(). Discussion on gh-3440.

rgommers · 2014-03-15T23:00:18Z

Failures fixed by avoiding usage of negative numbers in 727fa40.

rgommers · 2014-03-15T23:00:43Z

Thanks Pauli, this PR was massively helpful!

rgommers · 2014-03-15T23:01:36Z

@jnothman but it was failing on uint64. For signed types it indeed works fine.

rgommers · 2014-03-15T23:02:42Z

the test loops simply over supported_dtypes

Issue was that negative numbers were produced that were then going through astype(). Discussion on gh-3440.

rgommers · 2014-03-15T23:49:20Z

backported to 0.14.x, last commit is 90fbdd2.

jnothman · 2014-03-16T00:59:49Z

but it was failing on uint64. For signed types it indeed works fine.

Hmm... but the unsigned int case should be handled (it tests np.issubdtype(data.dtype, np.unsignedinteger) and returns a non-duplicated version (although we could possibly put in zeros instead).

rgommers · 2014-03-16T03:44:05Z

No, all that stuff is done in the self.spmatrix call at the beginning of test_astype, before the type casting. D is always complex, so that issubdtype check does nothing here. Then you get negative numbers in S, after which S.astype(np.uint64) fails.

Anyway, I fixed it already to not use negative numbers - those aren't necessary at all.

jnothman · 2014-03-16T07:49:54Z

Well, your solution could still produce negative numbers if there are
explicit zeros. You've still got the case there that stops testing for
unsigned int types; if we assume tests create no explicit zeros, then it
should be possible to test uints here too. I used negative and doubled
numbers in part to make the errors more obvious, but of course as a test it
matters little.

On 16 March 2014 14:44, Ralf Gommers notifications@github.com wrote:

No, all that stuff is done in the self.spmatrix call at the beginning of
test_astype, before the type casting. D is always complex, so that
issubdtype check does nothing here. Then you get negative numbers in S,
after which S.astype(np.uint64) fails.

Anyway, I fixed it already to not use negative numbers - those aren't
necessary at all.

Reply to this email directly or view it on GitHubhttps://github.com//pull/3440#issuecomment-37747919
.

rgommers · 2014-03-16T10:34:04Z

Yes, I did look at the test matrices to make sure that there were no negative numbers created for the choice I made. It's not the most robust fix if new tests are going to be added. My purpose was to get this PR in and get 0.14.0b1 out the door (building the binaries now). If you have a more future-proof solution then it's very welcome of course:)

pv added PR labels Mar 8, 2014

rgommers added this to the 0.14.0 milestone Mar 9, 2014

rgommers mentioned this pull request Mar 9, 2014

MacPorts builds yield 40Mb worth of build warnings #2944

Closed

pv added 12 commits March 12, 2014 21:45

ENH: sparse/sparsetools: replace SWIG by manual wrappers

4bc824a

BLD: sparse: update sparsetools build

875bb0a

MAINT: update gitignore re sparsetools

7b773a9

ENH: sparse: split sparsetools to smaller compilation units

5c66eea

ENH: sparse/sparsetools: better autogenerated code + comment the code…

8edd240

… better

MAINT: sparsetools: delete unused files

4d6e166

BLD: sparse: fix sparsetools depends

31b46cd

BUG: sparse: fix const declarations in generate_sparsetools

65d9f09

BUG: sparse: strip out forgotten things from generate_sparsetools

3b6134d

DOC: sparse: fix inaccurate comment in sparsetools.cxx

ba69087

BLD: sparse: check return value of generate_sparsetools.py on build time

f6778ee

TST: sparse/sparsetools: fix location for test_throw_error

c365990

rgommers and others added 4 commits March 12, 2014 21:45

BLD: fix Bento build after sparsetools wrapping changes.

5ab91a5

MAINT: rename scipy.sparse.sparsetools to _sparsetools to avoid problems

edbc2bf

Installing over a previous Scipy installation causes problems, as sparsetools is not a package any more. Therefore, change the name. At the same time, mark the module for internal use only.

WHT: sparse: pep8 fixes

3fc2d1f

DOC: sparse/sparsetools: fix errors in comments

c620455

rgommers added a commit that referenced this pull request Mar 15, 2014

Merge pull request #3440 from pv/c-sparsetools

5a626d2

MAINT: wrap sparsetools manually instead via SWIG

rgommers merged commit 5a626d2 into scipy:master Mar 15, 2014

rgommers added a commit that referenced this pull request Mar 15, 2014

TST: fix sparse test failures on Windows.

727fa40

Issue was that negative numbers were produced that were then going through astype(). Discussion on gh-3440.

rgommers added a commit that referenced this pull request Mar 15, 2014

TST: fix sparse test failures on Windows.

90fbdd2

Issue was that negative numbers were produced that were then going through astype(). Discussion on gh-3440.

rgommers mentioned this pull request Oct 10, 2015

ENH: Port upfirdn #5186

Merged

tmylk mentioned this pull request Oct 12, 2015

wishlist: sparse in-place AXPY/GEMM instead of deprecated sparsetools.csc_matvecs #5348

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAINT: wrap sparsetools manually instead via SWIG #3440

MAINT: wrap sparsetools manually instead via SWIG #3440

pv commented Mar 8, 2014

coveralls commented Mar 8, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

pv commented Mar 9, 2014

coveralls commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

pv commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

pv commented Mar 12, 2014

pv commented Mar 12, 2014

rgommers commented Mar 12, 2014

pv commented Mar 12, 2014

rgommers commented Mar 12, 2014

rgommers commented Mar 15, 2014

pv commented Mar 15, 2014

rgommers commented Mar 15, 2014

jnothman commented Mar 15, 2014

rgommers commented Mar 15, 2014

jnothman commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

jnothman commented Mar 16, 2014

rgommers commented Mar 16, 2014

jnothman commented Mar 16, 2014

rgommers commented Mar 16, 2014

MAINT: wrap sparsetools manually instead via SWIG #3440

MAINT: wrap sparsetools manually instead via SWIG #3440

Conversation

pv commented Mar 8, 2014

coveralls commented Mar 8, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

pv commented Mar 9, 2014

coveralls commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

pv commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

rgommers commented Mar 9, 2014

pv commented Mar 12, 2014

pv commented Mar 12, 2014

rgommers commented Mar 12, 2014

pv commented Mar 12, 2014

rgommers commented Mar 12, 2014

rgommers commented Mar 15, 2014

pv commented Mar 15, 2014

rgommers commented Mar 15, 2014

jnothman commented Mar 15, 2014

rgommers commented Mar 15, 2014

jnothman commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

rgommers commented Mar 15, 2014

jnothman commented Mar 16, 2014

rgommers commented Mar 16, 2014

jnothman commented Mar 16, 2014

rgommers commented Mar 16, 2014