Code Review: Documentation

[marlitas]

for https://github.com/phetsims/calculus-grapher/issues/268

Opening this issue to track documentation questions that feel too general for a REVIEW comment, and too small for their own specific issue.

[marlitas]

TransformedCurve

This is quite a heavyweight in the model. I am by no means fooling myself to insinuate that I tracked all of the math happening in this file, and to a certain extent must trust that @veillette and @pixelzoom have done their research here.

Overall the documentation was very thorough, which allowed me to follow the math in a very superficial manner helping to make the code legible. A couple general comments for this file:

• [x] I see the word weight everywhere, and though I know it helps indicate how a point should be calculated I've had a hard time actually putting my finger on what is being "weighed" and how. Because of how prevalent this is in the code and documentation, it feels like a more thorough explanation of weight would be helpful.
• [x] There is some reordering of functions that I think would be helpful for legibility, for example: grouping manipulateCurve and widthManipulatedCurve together, grouping createPedestalAt, createHillAt, createParabolaAt, etc. together. This just makes it easier to navigate the through the file and provides some order.

[marlitas]

I ended up not having too many REVIEW comments that fell into this category. The micro cases can be found by searching // REVIEW: in the code.

[veillette]

I'll assign this one to myself.

[pixelzoom]

Thanks @veillette. Looks like the remaining REVIEW comments are in your wheelhouse. I moved one to https://github.com/phetsims/calculus-grapher/issues/291, which I'll handle.

[veillette]

There is some reordering of functions that I think would be helpful for legibility, for example: grouping manipulateCurve and widthManipulatedCurve together, grouping createPedestalAt, createHillAt, createParabolaAt, etc. together. This just makes it easier to navigate the through the file and provides some order.

I attempted to reorder the methods, but I just realized that we have internal processes (from git-hooks?) that reorder methods behind the scene. Public methods are set above the private methods.
In spite of this constraint, I was able to reorder the methods in a way that can create a more natural grouping.

[pixelzoom]

... we have internal processes (from git-hooks?) that reorder methods behind the scene. P

That's a setting in the WebStorm "Commit Changes" dialog. I keep that turned off, because I don't like WebStorm deciding how my code should be arranged.

[veillette]

I added some context for weight functions, their purposes, and the various weight functions that we used in the sim in the header of TransformedCurve

We should note that the TransformedCurve class is the basis of the original curve, and, therefore,
its first, and second derivative will be evaluated. As a result, much effort was spent creating curve manipulations
that yields unusually smooth curves for which their first and second derivatives are themselves smooth.

Most curve manipulations make use of a weight function to "blend" in a curve segment into a previous curve.
A weight function is a mathematical device used when performing an average to give
some elements more "weight" or influence on the result than other elements in the same set.
The result of the application of a weight function is a weighted sum or weighted average.
A variety of weight functions ranging from gaussian kernel, super gaussian, mollifying functions
are used to create curves without cusps and discontinuities.

[veillette]

I have addressed all the REVIEW comments (except the one in GraphNode which is tracked in #291). Unfortunately I have removed the word REVIEW as I worked on them, so there is no easy way for @marlitas to track the work on this issue :( I'll mention that some of that documentation was vestigial, wrong and/or sometimes lacking. Thanks for pointing those out.

I also ordered the methods and added on comments on 'weight' in TransformedCurve :https://github.com/phetsims/calculus-grapher/issues/288#issuecomment-1470244786 .

Assigning back to @marlitas for review.

[marlitas]

These commits are very helpful and the reordering seems much cleaner to me! Thanks for clarifying and responding to the REVIEW comments. Closing this issue as completed!