Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce absstep to specify the stepsize to use close to zero #55

Merged
merged 1 commit into from
May 26, 2019

Conversation

andreasnoack
Copy link
Member

and rename epsilon_factor to relstep. Since we just introduced the epsilon_factor argument, I haven't deprecated it.

Setting the step size around zero can be important in applications and indeed I'm working on a project where the current default isn't a good choice.

The current version of this PR should have almost unchanged behavior for the current uses of the library. However, we might want to consider using a smaller absolute step size. This is effectively what https://cran.r-project.org/web/packages/numDeriv/numDeriv.pdf does and this can also be motivated by computing derivatives of log and abs2 around zero, see

Screen Shot 2019-05-25 at 20 25 45

Screen Shot 2019-05-25 at 20 27 32

Where "new" uses the square of the relative step size for the absolute step size. However, Pareto improvements only exist in books so some functions can lose on such a change.

Screen Shot 2019-05-25 at 21 10 20

though I'm not sure if new error is really worrying.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 43.868% when pulling da55575 on andreasnoack:an/abstol into 285af2a on JuliaDiffEq:master.

@codecov
Copy link

codecov bot commented May 25, 2019

Codecov Report

Merging #55 into master will not change coverage.
The diff coverage is 80.95%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #55   +/-   ##
=======================================
  Coverage   58.02%   58.02%           
=======================================
  Files           6        6           
  Lines         293      293           
=======================================
  Hits          170      170           
  Misses        123      123
Impacted Files Coverage Δ
src/jacobians.jl 82.75% <100%> (ø) ⬆️
src/finitediff.jl 23.07% <66.66%> (ø) ⬆️
src/gradients.jl 51.53% <75%> (ø) ⬆️
src/derivatives.jl 70.83% <83.33%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 285af2a...da55575. Read the comment docs.

returntype::Type{T2}=eltype(x),
f_x::Union{Nothing,T}=nothing;
relstep=default_relstep(fdtype, T),
absstep=relstep) where {T<:Number,T1,T2}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this default make sense? Does it recover the previous behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it recover the previous behavior?

Yes, it's to match the current behavior.

@ChrisRackauckas ChrisRackauckas merged commit 9489386 into JuliaDiff:master May 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants