JuliaMath
Added differentiation rule for mod2pi
This one seems subtle since this function only leaves x unchanged if you already accept the relevant equivalence classes. Probably still right, but I'd be interested to know if we're ok with this producing the "wrong" answer if you don't embrace the appropriate equivalence relation.
Good point. Is the name of the function (mod2pi) a sufficient logical guard against picking an inappropriate equiv relation?
That's ok with me. I'm mostly interested in how we think about testing these things. For example, if you used finite differencing to compare the results of symbolic differentiation, I think you'd get very strange disagreements for points that cross the 2pi boundary.
Would be interested to know what @simonbyrne thinks.
Is there a way that when you load the Calculus package, it could append notes like this one to the relevant docstrings?
If not, then I'd think a comment in the code and a mention in the package documentation would do the trick.
Thanks, Eric
On Wed, Mar 30, 2016 at 12:03 PM, John Myles White <[email protected]
wrote:
That's ok with me. I'm mostly interested in how we think about testing these things. For example, if you used finite differencing to compare the results of symbolic differentiation, I think you'd get very strange disagreements for points that cross the 2pi boundary.
Would be interested to know what @simonbyrne https://github.com/simonbyrne thinks.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/johnmyleswhite/Calculus.jl/pull/89#issuecomment-203504486
Eric Ford Professor of Astronomy & Astrophysics Center for Exoplanets & Habitable Worlds Center for Astrostatistics Institute for CyberScience Penn State Astrobiology Research Center Pennsylvania State University
Would it be reasonable to simply update the docstrings, as Eric suggested, and otherwise add the mod2pi() method as it stands? If there is still uncertainty as to what the optimal solution would be, shall we merge this for now, given that the functionality of this PR is needed? We can always revise it in due course, while being able to make use of the function in the meantime.