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

Does ForwardDiff need to be a dependency? #112

Closed
rafaqz opened this issue Jun 20, 2018 · 12 comments · Fixed by #121
Closed

Does ForwardDiff need to be a dependency? #112

rafaqz opened this issue Jun 20, 2018 · 12 comments · Fixed by #121

Comments

@rafaqz
Copy link

rafaqz commented Jun 20, 2018

Requires.jl could be used to remove this... I'm not sure its for commonly used functions anyway

@rafaqz rafaqz changed the title Does forward diff need to be a dependency? Does ForwardDiff need to be a dependency? Jun 20, 2018
@jverzani
Copy link
Member

Is there an issue with the dependency? It is heavy for the amount of use it gets here, so could possibly be removed in favor of some documentation.

@rafaqz
Copy link
Author

rafaqz commented Jun 20, 2018

It pulls in quite a few packages and slows load time and CI. when Roots.jl is otherwise very lean!

I was watching my travis build and realised how much of the stack was pulled in by that one dependency. Forward diff requires:

Compat 0.47.0
StaticArrays 0.5.0
DiffResults 0.0.1
DiffRules 0.0.4
NaNMath 0.2.2
SpecialFunctions 0.1.0
CommonSubexpressions 0.1.0

Thats half of the deps for one of my packages, and I don't use the functionality at all.

If you use a @require block you can keep Newton etc with ForwardDiff deps, they will just be exposed when ForwardDiff is actually loaded.

@rafaqz
Copy link
Author

rafaqz commented Jun 20, 2018

I'll put together a pull request if that helps

@jverzani
Copy link
Member

jverzani commented Jun 20, 2018 via email

@rafaqz
Copy link
Author

rafaqz commented Jun 20, 2018

Ok if that's a better option. Another option is Roots that need a jacobian (or whatever it is) could be in another package.

@KristofferC
Copy link
Member

Please not another package.

@ararslan
Copy link
Member

Also please don't introduce a dependency on Requires.

@rafaqz
Copy link
Author

rafaqz commented Jun 21, 2018

Just out of curiosity what wrong with a dep on Requires? I've started using requires to remove other deps, thinking that was an ok option?

@ararslan
Copy link
Member

It's a hack that's not really sound when it comes to precompilation. Better to wait until the language has first-class support for optional dependencies.

@rafaqz
Copy link
Author

rafaqz commented Jun 21, 2018

Thanks that is good to know. Optional deps will solve a lot of problems.

@rafaqz
Copy link
Author

rafaqz commented Jul 18, 2018

Requires seems to be a reliable solution for this kind of thing now: JuliaPackaging/Requires.jl#46

@jverzani
Copy link
Member

That's good to know. I still think it best to remove ForwardDiff and document (though I really want this for my private pedagogical issues, but it does seem too heavy to pull in during testing). This is next on my todo list for this package.

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 a pull request may close this issue.

4 participants