Avg scale #58
Avg scale #58
Conversation
|
I think this is a little nicer. Should be ready to merge, if someone wants to have a look |
|
The problem with that is there's no way for type inference to figure out what so you can ask for the dimension not just of a transformation but of a diagram, a path, etc. etc. |
| determinant. Proofs for the specified properties: | ||
|
|
||
| 1. (|det (scaling k)|)^(1/n) = (k^n)^(1/n) = k | ||
| 2. (|det t1|)^(1/n) * (|det t2|)^(1/n) * ... * (|det tn|)^(1/n) |
There was a problem hiding this comment.
There's no need to generalize the proof to t1 .. tn, just a product of two transformations still suffices. Only the exponent changes. In other words the dimension of the vector space has nothing to do with the number of transformations being composed.
There was a problem hiding this comment.
yes that was bone headed.
While we are at it, can we remove the wikipedia quote about the determinant. This is a basic fact from linear algebra, in fact it is often used to motivate the definition of the Jacobian.
There was a problem hiding this comment.
I don't understand your point. You want to remove a useful comment just because it is "basic"? What is it exactly that you don't like about having that comment there? Remember that some people (like me) will be in fact learning their basic linear algebra by working with the diagrams source! I struggled for a long time trying to figure out whether we could define something like avgScale with the right properties. Once I learned that fact about determinants everything became a lot clearer to me, so I copied that text there to help the next person trying to understand why the definition of avgScale is the right one.
There was a problem hiding this comment.
I didn't mean to offend anyone by calling this fact basic. It just seemed out of place. We have lots of mathematical facts in our library and very few (if any) others are explained. It is only in this context that I recommended removing it. Its isolated presence makes it stand out and gives me the impression that we think it is not well known. If you think readers of the source code will find it useful, then by all means lets leave it in.
There was a problem hiding this comment.
How about if we just put a single sentence something like "this works because the determinant is the factor by which a transformation scales area/volume" along with a link to the Wikipedia article?
There was a problem hiding this comment.
great, I'll take care of it.
There was a problem hiding this comment.
Also, perhaps we should explain more of the mathematical facts in our library. ;-)
There was a problem hiding this comment.
Yes, your probably right. Although, it's hard enough writing comments to explain the code sometimes :)
|
@byorgey s more general version of |
Moved avgScale to core