Rigid Transformations #39

carterbox · 2017-01-23T23:42:10Z

Add the rigid transformations: rotation and translation to the Polytope and Region classes. One method of specifying the rotation is incomplete. There is a TODO at the relevant place in the code.

…ate. Apparently convention is that object methods return a copy of the object and that methods are described using the noun form when doing so and the verb form when modifying in place. These functions have been renamed and altered to return copies to reflect this convention. The helper functions have been hidden from the public API using underscore.

carterbox · 2017-01-23T23:58:02Z

The tests for rotation fail when cvxopt is used as the solver because the array storing chebXc is returned as 2D array instead of a 1D array.

assert_allclose(p.chebXc, [-0.5, -0.5])
(shapes (2, 1), (2,) mismatch)
 x: array([[-0.5],
           [ 0.5]])
 y: array([-0.5,  0.5])

When numpy is used as the solver, chebXc is a 1D array.

`cvxopt` returns a 2D array whereas `scipy` returns a 1D array. 1D arrays are more versatile because they don't need transpositions.

slivingston · 2017-01-29T15:46:30Z

I am reviewing this now and will send comments within 1 hour.

slivingston · 2017-01-29T16:37:28Z

regarding 79d85ec in which you use numpy.squeeze to make the return values of the LP solvers have the same shape, I agree with your proposed change. After changes from this PR are merged, I want to simplify other parts of the source code accordingly; e.g., in projection_iterhull. There is some risk that external code that uses polytope.lpsolve will break. However, such code is fragile anyway because it only would have been able to manipulate which LP solver is used by re-assigning to the module-level variable lp_solver. (N.B., issue Choosing between scipy and cvxopt solvers #17 concerns a method to select different solvers.)
Why do you use the prefix _ in the function names _rotate and _translate? They seem to be funtions that are worth including among exported names.
In the docstring of _rotate, should "indicies" be changed to "indices"? Similarly, in the call to logger.info on line 499 the message has "indicies".
In _rotate, you write "This function has two different ways to specify this plane", but three methods are given.
The function arguments of _rotate should be explicitly referenced in the docstring. E.g., write that the rotation matrix of method (3) is R.
Is there some intuitive explanation for the arrangement of parameters of _rotate? Method (1) is performed beginning with seems call of the form _rotate(P, u, v, theta). However, method (3) seems to require some value for the parameters u and v, but ignores them in favor of R.
The R parameter of _rotate is not available through rotation. Should it be?
Is the pass in the first conditional block of _rotate meaningful? From the language specification, it is not necessary because there is already a call to logger.info before pass.

carterbox · 2017-01-29T17:41:06Z

There are two main themes in your comments.

First, why are _rotate and _translate not public? (2) My intent is for users to call the class methods instead of the module level helper functions.

Second, why is the third method of rotation undocumented and hidden? (4,5,6,7) The third undocumented method is hidden because it is not safe. Theoretically, you could provide any NxN matrix to the R parameter, but there are no checks made to determine whether it is a valid rotation (rigid transformation). If such a non-rigid transformation was provided, the function would not be rotating the polytope. Thus, the third method is a secret method which is only used for efficiently passing rotations from regions to subregions. As your comments suggest, I should probably not leave this third method undocumented, but instead slap a warning on it for the reasons stated above.

Also making parameters u and v keyed parameters is a good idea that I should do.

The third case for defining a rotation, supplying a matix, was undocumented. Now it is documented with epytext. Additionally, the u and v parameters are now keyed, and the function enforces valid combinations of parameters.

slivingston · 2017-01-31T18:49:37Z

Thanks for explaining about the hidden method and why _rotate and _translate should not be public. The docstring of _rotate is good, but it is not readily apparent (without studying source code) how to find it or its relationship with the rotation class methods. The same comment applies to _translate, but its docstring is not much different from the class methods translation.

Do you recommend an approach to bridge the _rotate and rotation documentation gaps?

Three ideas:

Refer to the private module-level method directly. In Epydoc syntax, e.g.,
```
Consult L{polytope.polytope._rotate}, which this method calls
internally, for details about usage.
```
but then there is risk of confusion: why direct users to a private function that they should not call directly?
Make the module-level method public (in particular, remove the _ prefix), but clarify that most users will want to use the class methods.
Move all currently relevant parts of the docstring of _rotate to the docstring of Polytope.rotation, and then provide a reference to Polytope.rotation in the docstring of Region.rotation. Notes about not implemented stuff in _rotate can just stay in the docstring of _rotate.

carterbox · 2017-02-02T01:18:03Z

I've been thinking about how I've created two separate rotation interfaces. One has the third option and the other doesn't. My reason for hiding the third option is that it is unsafe, and I don't trust the user to provide a correct rotation matrix. However, what if the user wants to perform non-affine transformations to the polytopes? In that case, they should be able to supply any matrix they want.

The solution is to add the capability of providing an arbitrary (N+1, N+1) size transformation matrix (includes homogenous coordinate). I have thought of two ways to implement this idea. In both cases adding the explicit transformation matrices allows transformations to be chained together or used repetitively.

Option A

polytope.Polytope.translation calls module function, returns copy of polyreg
polytope.Polytope.rotation calls module function, returns copy of polyreg
polytope.Polytope.transformation calls module function, returns copy of polyreg

polytope.translate translates polyreg in place, returns transformation matrix
polytope.rotate calculates rotation matrix, rotates polyreg in place, returns transformation matrix
polytope.transform transforms polyreg in place, returns transformation matrix

The purpose of having two functions for each operation is as follows:

Can save one step by not forcing the user to generate a transformation matrix
Supports workflows with invariant polyregs or static polyregs

Option B

polytope.Polytope.transformed calls module function, returns copy of polyreg

polytope.translation. calculates translation matrix, returns transformation matrix
polytope.rotation. calculates rotation matrix, returns transformation matrix
polytope._transform. transforms polyreg in place, returns nothing

The purpose of having only one function for each operator is as follows:

Option A breaks the Zen of Python because it creates multiple ways of doing the same thing. Option B forces only one way to transform a polyreg.

carterbox · 2017-02-02T01:56:39Z

Also, what about moving these transformations to their own sub-module?

johnyf · 2017-02-02T04:44:47Z

polytope/polytope.py

@@ -290,6 +290,32 @@ def intersect(self, other, abs_tol=ABS_TOL):

        return reduce(Polytope(iA, ib), abs_tol=abs_tol)

+    def translation(self, d):
+        """Translate the Polytope by a given vector.


"by a given vector" leaves argument d unexplained until one reads @param d: The translation vector. It is more direct to write: "by the vector d". (or C{d}...). We can then omit @param d later, so the user has to read less.

"the Polytope" could be replaced with "self". This is precise, survives subclassing (a subclass of Polytope could be called MagicBox), and is shorter.

"Translate" (a verb in active voice) could lead a user to think of in place modification, and miss the @return at the bottom of the docstring. Conversely, the user should read the entire docstring to obtain the necessary information (this docstring is short, but I mean for other cases too).
This situation can be avoided by writing:

"Returns a copy of self translated by the vector d."

We can then omit @return and @rtype, because @return is now part of the docstring's title, and @rtype is communicated by "copy of self".

Putting these changes together, we obtain a shorter docstring:

"""Returns a copy of C{self} translated by the vector C{d}. Consult L{polytope.polytope._translate} for implementation details. @type d: 1d array """

We could also make d more specific by naming a suitable type of numpy array.

Also, I would recommend for each argument x, to write @param x before @type x, because one is firstly interested in what a variable means, and secondly in what type it has. Python is untyped anyway, and using isinstance is discouraged, so @type is secondary information. The function should process any duck that fits the description.

We could regard @type as a "type hint", with the meaning described in Lamport and Paulson, "Should your specification language be typed?". Type hints have reached Python itself.

Under the convention that @type is a type hint, and not a rigid requirement, we can then be more specific in what type we mention (for example set, instead of "container"), thus demonstrating our typing intent with an example type, knowing that the user will not think of the example as a restriction.

I agree with all of the comments and recommendations above, but I did not consider it required before this PR would be accepted because the meaning of "vector" can be inferred. Instead, we might return to docstrings later during quality assurance review of documentation.

Alternatively, the docstring that @johnyf pitches above is good for me, too.

johnyf · 2017-02-02T04:47:00Z

polytope/polytope.py

+        return newpoly
+
+    def rotation(self, u, v, theta=None):
+        """Rotate the Polytope. Only simple rotations are implemented at this


The arguments u and v are undefined.

Are "simple rotations" rotations in three-dimensional Euclidean space?

I think that currently, "simple rotations" means "easily implemented rotations" or "rotations that can be achieved in one step".

I've read that for N dimensions, the path between any two orientations will require at least 1 and at most floor(N/2) rotations, so for N > 3, there is now a multi-step transformation required for arbitrary reorientation. I'm not sure yet why the rotation cannot be achieved in one step; since, by definition, all the motion can be projected into a plane. I'll have to do more reading when I implement the missing rotation method.

Finding how to phrase this restriction in a mathematically precise way would be informative for the reader of the docstring.

I am not sure I understand what the path between two orientations is. If we are interested in a function that maps the ambient Euclidean space to itself, so that points on the object in its initial configuration be mapped to "the same points on the object" (using coordinates fixed wrt to the object) in its final configuration, then such a function seems to always exist (we could multiply the transformation matrices).

It seems to depend on how the mapping is parameterized. If the mapping is expressed in terms of rotations, then an example that appears to require at least two rotations is a pen horizontal on a table, and the same pen vertical, but with its cap also rotated by 90 degrees around the pen's longitudinal axis. It may not be the case that any mapping from one orientation to another can be represented by a projection onto a hyperplane.

A reference that could be useful for studying three-dimensional transformations is Chapter 2 from the book "A mathematical introduction to robotic manipulation".

I might be missing something, but I think that the comment above that u and v are not defined is redundant with my earlier comment that the documentation of hidden function _rotate should be available from that of the class method rotation.

My comment on u and v is redundant, I noticed it after posting.

Yes, a more mathematically rigorous description would probably be helpful.

When I say path, I mean "the series of simple rotations required to reorient from one orientation to another." By simple rotation, I revise my earlier definition to "a rotation that may be described by only one plane of rotation". These simple rotations are, as you said, reorientations that may be projected into a hyperplane. It is useful to also define compound rotation, "a rotation that has multiple planes of rotation and is the composition of multiple simple rotations".

Although Euler's rotation theory says that all reorientations in 3D are simple rotations, for N > 3 this is no longer true. As you said, the parameterization limits the capability of the mapping, and I believe that the current input parameters are not capable of expressing the compound rotations. In other words, the parameters of this function are only capable of expressing simple rotations at this time.

I guess the phase "at this time" is misleading. Will this function be expanded to include compound rotations in the future? No. I think it is better that only simple rotations are implemented, and users can perform compound rotations by applying a sequence of simple rotations.

Does this make sense? I will be using this explanation to clear up the docs.

Yes, your explanation makes sense.

Though I am not aware of one yet, there might eventually be motivating use-cases for handling compound rotations. So, I do not regard "at this time" as misleading.

johnyf · 2017-02-02T04:50:25Z

polytope/polytope.py

@@ -442,6 +468,134 @@ def text(self, txt, ax=None, color='black'):
        _plot_text(self, txt, ax, color)


+def _translate(polyreg, d):
+    """Translate a polyreg by a vector in place. Does not return a copy.


"Modifies C{polyreg} in place" provides more information, and emphasizes that the object will change. Both "in-place" and "in place" appear in Python's own documentation.

From the context of those links, I think what you're telling me that "in-place" means "without moving" or "without a copy", and "in place" means "instead" such as when used as "in place of".

Indeed, I was too quick to link to the second page. This "in place" from Python's documentation is used in the sense of "in-place" (on the same instance).

To clarify my intent, I am citing both forms to indicate that I don't know which one is more widely used. Wikipedia uses the hyphenated version. Your comment suggests that "in-place" may be a better choice.

I am happy with both "in-place" and "in place". Given there is no reliable distinction that we have found, we can wait for empirical feedback from users.

I did not consider it critical, but since we are debating "in-place" vs. "in place", I will add that "Does not return a copy" is incomplete. It should be given a subject, changed to the passive voice (e.g., "A copy is not returned."), or deleted because it is redundant with the previous sentence.

I agree with keeping the first sentence because it suggests the second sentence.

johnyf · 2017-02-02T04:54:34Z

polytope/polytope.py

+def _rotate(polyreg, u=None, v=None, theta=None, R=None):
+    """Rotate this polyreg in place. Return the rotation matrix.
+
+    Simple rotations, by definition, occur only in 2 of the N dimensions; the


I see that my guess above about what "simple rotation" means was off. Function _rotate is hidden, so some (minimal) mention in the docstring of method Polytope.rotation of what kind of rotations are supported would help a user.

johnyf · 2017-02-02T04:58:52Z

tests/polytope_test.py

+
+    p = p.rotation(0, 1, np.pi/2)
+    print(p.bounding_box)
+    assert(p == p2)


I, too, used to enclose the asserted expression within parentheses. The parentheses can be omitted, because in Python assert is a statement, not a function (in contrast for example to print, which was a statement in Python 2 and became a function in Python 3).

just to be clear in terms of reviewing this PR, @johnyf are you requesting that the parens be deleted, or are you only making a comment about style that is OK either way (with or without parens)?

The main reason I made this remark is because it took me some time before I noticed that assert is a statement and doesn't need parentheses. I am not requesting this change for merging, though it would be nice to apply it.

I'm not going to change this because all of the other tests use assert as a function. They can all be changed together in some other pull request.

Those parentheses are mine. As I mentioned, at some point I noticed that assert is a statement. The parentheses can be removed after merging.

johnyf · 2017-02-02T05:23:43Z

YAGNI may apply. Allowing the user to pass matrices in the hidden interface seems to me enough for now, until usage over time suggests the next step, if any.

Otherwise, we enter a discussion about representing rotations, should we use rotation matrices, some other representation (quaternions or higher-dimensional analogs, etc), and so what should be the "default" interface. As @carterbox already noted, multiple interfaces are challenging PEP 20's "There should be one-- and preferably only one --obvious way to do it." I should emphasize the caveat "obvious" here. The hidden matrix interface is not-so-obvious, and it may be fine to leave it so, until experience shows otherwise.

Humans would probably be happier to pass angles and vectors as arguments. Machines and developers of code for robotics applications probably like transformation matrices (or other representations) more. Option A seems to me more usable by humans, taking into account that "Although practicality beats purity." (PEP 20).

In option B, I'm not sure what arguments polytope.Polytope.transformed would take (a transformation matrix probably). Naming this method transformation would be in accord with the reasoning behind names in Option A (verbs vs nouns).

I support keeping everything within the module polytope. This module already contains almost everything, so it is more uniform to let it remain so. Eventually, I would like to see the package organized into smaller modules, but:

I do not know any "canonical" way to split the module, so I cannot suggest one. Coming up with an answer to this architectural question seems to require a clear view of the entire module. Splitting only one part elsewhere and keeping the rest together could confuse a future developer. This would need to be discussed. Elsewhere, it has worked fairly well for me to keep each "copy" of a theory in its own module (e.g., dd.bdd vs dd.cudd).
it is easy to have everything polytope-related in a single module, so that almost all API changes can be applied by searching though one file (since there are API changes stashed on branch overhaul--unfortunately this keeps popping up).

It seems that the polytope.polytope was intended to be polytope.Polytope (because we do have a polytope.polytope namespace...).

slivingston · 2017-02-03T23:36:52Z

I think that both options have worthy merits. @carterbox if you find that one option is better for the application that originally motivated you to do this work, can you share a distilled example here? It can be a sketch of code, i.e., not necessarily valid Python.

Regarding creating a new transforms module, I agree with comments from @johnyf.

Translation method docs edited for brevity using markup to link docs. Rotation method docs rewritten to more clearly define what types of rotations can be expressed through the given parameters. Documentation was added to the class method rotations.

carterbox · 2017-02-04T21:47:11Z

In summary, I am adopting the a policy of YAGNI. I have not changed the original structure of methods, but I have updated the documentation make it (hopefully) more helpful. I believe that with the updated documentation, the hidden methods will not need to be referenced in order to use the class methods which call them.

After some further reading, defining the rotation angle as twice the angle between the two provided vectors removes ambiguous cases such as collinear vectors facing opposite directions.

slivingston · 2017-02-05T02:10:47Z

polytope/polytope.py

@@ -310,7 +310,7 @@ def rotation(self, u, v, theta=None):
        (1) u and v are the indicies (0, N] of two basis vectors, and theta is
        the angle of rotation.

-        (2) u and v are two unit vectors, angle of rotation is the angle
+        (2) u and v are two vectors, angle of rotation is TWICE the angle


Can you provide citations? However, this might not be important because it is not implemented yet.

As this website explains, if you use full angles instead of half angles, there is ambiguity when rotations are pi.

slivingston · 2017-02-05T02:14:56Z

polytope/polytope.py

+        Use one of two methods that to describe the plane of rotation and the
+        angle of rotation (in radians) with u, v, and theta.
+
+        (1) u and v are the indicies (0, N] of two basis vectors, and theta is


should that be [0, N)? I am also happy with [0,N[.

or 0..N-1 (as in TLA+)

I think that 0..(N-1) or 0..N-1 are correct, but [0, N) is ambiguous, because the indices should be integers. In a typed formalism, one could claim to have declared that u, v are of "integer type" (whatever that may mean). However, in polytope we have neither type declarations of any form, nor are integers the norm. So, I think that 0..(N-1) avoids this ambiguity. Explicit is better than implicit (PEP 20).

In a context that involves real numbers, I prefer [0, N) to [0, N[, because I find it easier to distinguish a a closing parentheses ) from a closing bracket ], than an opening bracket [ from a closing bracket ]. I do appreciate the symmetry of the latter notation though.

I believe that "indicies" is a typo. Both "indices" and "indexes" appear to be used as plural form of "index". A remark about the same was made earlier.

Also, I think that if one method is supported now, then No.2 should be omitted. Why have the user read two items, and then find out the 2nd isn't available?

I am opposed to the polymorphism of u, v. I had to read No.1 multiple times to convince myself that I understand correctly. u, v should better have one type in each signature. If they are integer indices, it is more familiar to name them i, j or m, n.

"basis" could be ambiguous. We could specify that the basis is the same as that used to express the coordinates.

Relevant for those interested in untypeness: omega is in transition from typed to untyped (cf tulip-control/omega#3).

Special cases aren't special enough to break the rules [PEP 20]. It seems more uniform to let u, v be vectors. This way:

the interface remains unchanged when the implementation is extended to handle arbitrary vectors

an UnimplementedError is raised if vectors outside the intended basis are passed

users don't have to think about any particular basis

index-based thinking is avoided. This is more Pythonic and more geometric / structural.

Defining the basis vectors u0, u1 etc in user code is relatively simple, and it also names them, so it is probably easier to read (as opposed to some integers 0, 1, ...).

@slivingston That is correct, I meant to type 0..N-1.

@johnyf "indicies" is a typo. I keep spelling it wrong. I agree that polymorphic u, v probably isn't the best. However, I'm not going to get rid of rotation by indexed basis vector because the current implementation is based around that special case. What I will do is rename the parameters to i, j, and hide the second method. When I figure out how to implement the second option, the parameters u, v will reappear.

What I had in mind was raising an error if either u or v isn't a basis vector. If both are basis vectors, then convert them to indices, and proceed with the rest of the computation as currently done. Your current decision yields a simpler interface, and simple is better than complex (PEP 20).

slivingston · 2017-02-05T02:23:01Z

polytope/polytope.py

+    be parameterized by its plane of rotation. Compound rotations are the
+    combination of multiple simple rotations; they have more than one plane of
+    rotation. For N > 3 dimensions, a compound rotation may be necessary to map
+    one orientation to another (Euler's rotation theory no longer applies).


would "Euler's rotation theorem" be more appropriate here, rather than "Euler's rotation theory"?

slivingston · 2017-02-05T02:28:15Z

I made three minor comments in situ. Once you address those, I think that this is ready for merging.

@johnyf do you have any more comments?

slivingston · 2017-02-06T03:41:54Z

polytope/polytope.py

+    @param u: The first vector describing the plane of rotation.
+    @type u: 1d array
+    @param u: The second vector describing the plane of rotation.
+    @type v: 1d array.
    @param theta: The radian angle to rotate the polyreg in the plane defined
                  by u and v.


Should the description for theta be updated to "...defined by i and j" ?

Good catch! I have fixed this.

slivingston · 2017-02-06T03:49:34Z

With the introduction of i and j, _rotate now has three disjoint sets of parameters for three different kinds of invocation. It is OK to let it remain so for this pull request because it is hidden and only one parameter set is available from the corresponding class methods. However, eventually when implementing the other expressions of rotation, we should consider splitting _rotate.

slivingston · 2017-02-06T04:09:11Z

I think that there are good arguments both directions: from @johnyf in #39 (comment) in support of uniformly using vectors, and from @carterbox in #39 (comment) observing that the current implementation is in terms of standard basis indices.

I made one more comment about documentation: #39 (comment). Once it is addressed, I recommend accepting this pull request.

Uniformity of function signatures and type traits can be provided later when the rotation implementation is extended. Committing to the index case now implies risk of API breaks later. However, without the other invocations of _rotate available, I find it hard to confidently secure the API for rotations.

Broke the u, v parameter for the rotation function into two separate parameters: i, j for the identity basis vector indices and the u, v for the vector specification.

johnyf · 2017-02-06T09:03:43Z

I agree that this PR can be merged. If there are any modifications left to make, they can be applied after merging.

About the function signature, simple is better than complex, so using indices as arguments may be more accurate for now. Given that the API is scheduled to see extensive changes, the change to using vectors can be lumped into the other changes (assuming it is implemented).

pull request #39 #39 Changes are from branch `affine` of https://github.com/carterbox/polytope.git

carterbox added 4 commits January 20, 2017 15:40

ENH: Add rotate function to the Region and Polytope.

324b850

ENH: Add translate function to Polytope and Region.

af401d8

DOC: Reformat documentation, TODO, and log messages for rotate.

aff57bf

carterbox mentioned this pull request Jan 23, 2017

Rigid Rotation / Reorientation #32

Closed

slivingston self-assigned this Jan 23, 2017

carterbox mentioned this pull request Jan 27, 2017

bounding_box format #34

Open

BUG: Change cvxopt.glpk output to match scipy solver output.

79d85ec

`cvxopt` returns a 2D array whereas `scipy` returns a 1D array. 1D arrays are more versatile because they don't need transpositions.

DOC: Improve _rotate docs for third case.

cb9e9ed

The third case for defining a rotation, supplying a matix, was undocumented. Now it is documented with epytext. Additionally, the u and v parameters are now keyed, and the function enforces valid combinations of parameters.

DOC: Add epydoc to translation functions.

bf65d33

johnyf reviewed Feb 2, 2017

View reviewed changes

DOC: Change two vector rotation angle specification.

173106e

After some further reading, defining the rotation angle as twice the angle between the two provided vectors removes ambiguous cases such as collinear vectors facing opposite directions.

slivingston reviewed Feb 5, 2017

View reviewed changes

DOC: Fix some typos in rotation docs.

65245d9

slivingston reviewed Feb 6, 2017

View reviewed changes

API: Change u, v to i, j in rotate functions.

5d4b036

Broke the u, v parameter for the rotation function into two separate parameters: i, j for the identity basis vector indices and the u, v for the vector specification.

slivingston merged commit 5d4b036 into tulip-control:dev Feb 7, 2017

slivingston added a commit that referenced this pull request Feb 7, 2017

Merge PR #39 "Rigid Transformations"

9edf872

pull request #39 #39 Changes are from branch `affine` of https://github.com/carterbox/polytope.git

carterbox deleted the affine branch February 7, 2017 03:07

johnyf pushed a commit that referenced this pull request Apr 19, 2017

Merge PR #39 "Rigid Transformations"

907934a

pull request #39 #39 Changes are from branch `affine` of https://github.com/carterbox/polytope.git

johnyf added the enhancement A new feature, an improvement, or other addition. label Apr 30, 2017

Rigid Transformations #39

Rigid Transformations #39

Conversation

carterbox commented Jan 23, 2017

carterbox commented Jan 23, 2017 • edited Loading

slivingston commented Jan 29, 2017

slivingston commented Jan 29, 2017

carterbox commented Jan 29, 2017 • edited Loading

slivingston commented Jan 31, 2017

carterbox commented Feb 2, 2017 • edited Loading

Option A

Option B

carterbox commented Feb 2, 2017

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

slivingston Feb 3, 2017 • edited Loading

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

johnyf Feb 2, 2017 • edited Loading

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

johnyf commented Feb 2, 2017

slivingston commented Feb 3, 2017

carterbox commented Feb 4, 2017

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

carterbox Feb 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slivingston commented Feb 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slivingston commented Feb 6, 2017

slivingston commented Feb 6, 2017

johnyf commented Feb 6, 2017

carterbox commented Jan 23, 2017 •

edited

Loading

carterbox commented Jan 29, 2017 •

edited

Loading

carterbox commented Feb 2, 2017 •

edited

Loading

slivingston Feb 3, 2017 •

edited

Loading

johnyf Feb 2, 2017 •

edited

Loading

carterbox Feb 5, 2017 •

edited

Loading