-
Notifications
You must be signed in to change notification settings - Fork 13
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
Using DifferentiationInterface in SymbolicRegression #319
Comments
Indeed that used to be a widespread approach, but a
Why is that? If they use Zygote.jl as the backend, it will be installed anyway. And AD backends are infinitely heavier than ADTypes.jl or DifferentiationInterface.jl. ADTypes.jl has zero dependency on Julia >= 1.9, and DifferentiationInterface.jl is very lightweight too. Besides, if you use them internally, they'll be in the Manifest.toml anyway.
To me, the following interface is even better, because it does not introduce a separate using ADTypes, SymbolicRegression, Zygote
options = Options(AutoZygote()) |
And by the way, I'd be thrilled to help you get DifferentiationInterface.jl up and running with SymbolicRegression.jl. Don't hesitate to ping me for reviews or advice in that PR |
Thanks! It's not so much the extra dependency as SymbolicRegression.jl would of course already depend on DifferentiationInterface.jl if this goes through. The issue is I would be asking beginner users to install a third package in their top level environment, even after I have already asked them to install the AD backend package itself, just so they can set one of the parameters of the search.
I agree with this, in the example I gave above I allow both using DifferentiationInterface: lookup_backend, AbstractADType
function Options(autodiff_backend::Union{AbstractADType,Symbol})
backend = autodiff_backend isa Symbol ? lookup_backend(autodiff_backend) : autodiff_backend
return Options{typeof(backend)}(backend)
end So advanced users can install ADTypes and customize the backend as they desire, but beginner users (which SymbolicRegression.jl gets a lot of from the Python frontend PySR) still can just pass in a (Even package extensions themselves I find to be a distraction for beginners, which is why PySR just automatically installs stuff as the user needs it – https://github.com/MilesCranmer/PySR/blob/89e991d9d7a5e34025bdbbe9076a8fd0892f04cc/pysr/julia_extensions.py#L8-L36 – rather than asking the user to install each package themselves. I just find it to be another barrier for people using my package.) |
I understand your point, and I do think it makes a lot of sense to offer an easier setting, especially for people coming from Python. But I don't think this The best of both worlds might be to automatically turn a julia> using ADTypes
julia> s = :Zygote
:Zygote
julia> auto_s = Symbol(:Auto, s)
:AutoZygote
julia> backend = @eval $auto_s()
AutoZygote()
julia> backend isa AbstractADType
true There might be some subtleties as to which |
The reason I thought it might make sense to offer such a mechanism in this package is so that we can avoid having different ways of solving this. I am guessing that other packages may want to offer similar functionality in the future. So it would be nice to have a unified mapping from symbol to backend type. Even something like function Auto(key::Symbol; options...)
backend_type = lookup_type(key)
return backend_type(; options...)
end Which would let me pass extra backend configuration options from the user. Then I can import this function internally in my package, and allow the user to pass a symbol and autodiff configuration to my options constructor, without needing the user to manually install |
I don't think a unified mapping from |
I guess I feel like You could even make it like function Auto{key}(; options...)
backend_type = lookup_type(key)
return backend_type(; options...)
end rather than needing So, to summarise my thoughts: The issue with having the user install ADTypes.jl themselves is that it presents a small barrier to usage and more mental overhead. While it wouldn't slow down performance, it would mean another line in the user's Project.toml file, the extra effort of needing to run The two workarounds are:
|
From my POV, this is why ADTypes exists in the first place–it is the standard you are asking for. ADTypes are basically symbols with optional configuration options:
As Guillaume mentioned, there are some very large packages putting their weight behind ADTypes as a unified interface to select backends:
Maybe reexporting ADTypes is an option if you don't want users to have to load the package explicitly? (EDIT: just saw you addressed this simultaneously.) |
@adrhill oops, our messages crossed. Regarding this:
It's a bit tricky, because either I:
|
My issue is that if I make this translation available in DI (or in ADTypes itself for that matter), I somehow endorse it and encourage people to use it. But I don't want to, because there should only be one standard. Additionally, starting from a Symbol makes everything else type-unstable, unlike a properly defined AD backend. |
Additionally, if you ever want to use our second order or sparse AD functionality1, the amount of symbols will grow combinatorially. Footnotes
|
Actually this wouldn't be an issue. First, this wouldn't redefine anything, it's just making an mapping that packages can use for easy user-side APIs. But a user would always be able to pass in a full ADType instead to the options if they wish. Second, if the symbol is fixed in some code, Julia would just inline it and it would be stable.
This also wouldn't be an issue – the symbol here would only be the type specification. Like this: function Auto(symbol, options...; kws...)
if symbol == :Zygote
AutoZygote(options...; kws...)
elseif symbol == :Enzyme
AutoEnzyme(options...; kws...)
end
end So, in your example, it would be Auto(
:Sparse,
Auto(:ForwardDiff);
sparsity_detector=TracerSparsityDetector(),
coloring_algorithm=GreedyColoringAlgorithm(),
) which could be done without needing to re-export all the ADTypes. |
By the way, just curious, why not have const AutoZygote = Auto{:Zygote}
const AutoSparse = Auto{:Sparse}
const AutoEnzyme = Auto{:Enzyme} instead of Auto{:Sparse}(
Auto{:ForwardDiff}();
sparsity_detector=TracerSparsityDetector(),
coloring_algorithm=GreedyColoringAlgorithm(),
) so you don't lose extensibility. I mean in general I should say I don't feel too strongly about this. It just seems like it would be pretty useful for designing user-facing APIs. I note that will have to do some symbol => type mapping anyways, because of PySR (users can't specify Julia types there). I felt like it would be bad practice to come up with my own AD symbol => AD type mapping when others may want to use one as well (not to mention there might be more AD types in the future, so should probably be centralized and kept up-to-date). |
You do realize that this is not exactly diminishing the mental load of the user ;) This is essentially reinventing ADTypes with symbols.
For the same reason that symbols are not specific enough, symbol wrappers won't work either. I encourage you to take a look at the ADTypes documentation if you want to understand what we actually put inside each backend struct, and why it matters. |
To me this is the main (or only) argument in favor of symbols: calling Julia autodiff from Python or other languages. Everything else can be solved from within Julia with minimal efforts, but this seems to be a hard limit. |
Although even that limit can be overcome with from juliacall import Main as jl
jl.seval("using ADTypes")
backend = jl.AutoZygote() |
Sorry I was just giving that as an example in regards to the comment that "amount of symbols will grow combinatorially". For that example in particular you would probably just want the user to install ADTypes, so that they can get really custom with it. Which my request is not opposed to! Advanced users are not as sensitive to mental overheads :) It's really just the newbies where it would be great to have shortcuts for. In the cases where the user is just selecting a backend like enzyme or zygote or forward diff, just allowing a symbol would be nice. And then for the reasons to solve this here rather than elsewhere, I reference my comments in #319 (comment).
Indeed it can. But,
|
Sorry I am going off topic here but couldn't you just do this as follows? struct Auto{S,C} <: AbstractADType
options::C
Auto{S}(; options...) = (nt = NamedTuple(options); new{S,typeof(nt)}(nt))
end
Base.getproperty(a::Auto, k::Symbol) = getproperty(getfield(a, :options), k)
const AutoZygote = Auto{:Zygote}
const AutoEnzyme = Auto{:Enzyme}
... Since |
That's actually not a bad idea for ADTypes.jl in the long run. In the meantime I opened SciML/ADTypes.jl#62 to see what other maintainers think. Can you play around with that branch in your DI integration attempt, and let me know how it works? Set ADTypes compat to 1.3.1 to be sure that you get the right version. |
Sounds good to me |
Apart from this issue, which is getting solved, any other troubles when trying to integrate DifferentiationInterface.jl? What are you using it for? |
Basically just to let the user choose what AD backend is used during constant optimization. The only issues left are just from the different backends being incompatible (only Zygote/ChainRules works at the moment), but this is the fault of other backends, not the interface. (I’m not sure if Enzyme will work though, as usually I would have to declare what variables are being used as mutable storage?) |
Note that for some backends you'll get an epic performance boost by using preparation, especially in small dimension. What is the typical number of constants you optimize?
Does ForwardDiff work at least?
Well, the whole point of DifferentiationInterface is that you get to try it out for free! Related discussions: |
I usually can't use preparation because the gradients change for every expression. But the number of constants is typically ~1-10. For parametrized expressions (the reason I'm adding Zygote support) it can be up to 100.
For ForwardDiff I unfortunately have to use a separate function because it requires changing the element type.
Yeah, indeed it seems hard to do so. I get the following when I try –
I wonder if there's some generic way to declare storage in a way that would pass the required info to Enzyme as well as any future backend that might require it? Otherwise will need to have an if-statement over backend which seems non-optimal. Especially because performance is so key to my use-case. |
Symbol
?
I'm curious now, why can't your code be type-agnostic?
Do you have a full reproducible example?
This is related to, but different from, the issue about supporting active and inactive variables. And it's exactly what I said about Enzyme earlier: it is so different from other backends, and offers so many more options, that if I want to include every one I'll turn DI into a shallow frontend for Enzyme only. It's a question I haven't solved yet. |
@MilesCranmer I'm revisiting old issues and I think the one feature you're missing corresponds to #551? |
Cool! |
Alright then, I'm gonna close this issue in favor of the more specific #551, don't hesitate to open new ones! |
Hi @gdalle,
Awesome package, thanks for making it!
I am considering whether to use this as the backend interface of SymbolicRegression.jl to allow the user to pick an AD backend (current attempt). However, one thing I was wondering if you would consider is designating a mapping from
Symbol
to the correctAbstractADType
? I could implement this within my package, but it seems like something that should live here for others as well.Basically, I would prefer that my users do not need to install both Zygote.jl and DifferentiationInterface.jl (or
ADTypes
) in their environment to set Zygote as the backend. I would rather they just pass, e.g.,:Zygote
and then have my package, which depends on DifferentiationInterface.jl, be able to internally map to the rightAbstractADType
.Within DifferentiationInterface.jl this could look like:
Then within my package, I can have my
Options
constructor look like this:which means the user has a more lightweight interface; they can just pass the symbol:
And then I map to the right type internally.
I guess I could also re-export all the abstract types but it feels a bit overkill. That would also mean I would be unable to provide helpful error messages if they forget the name of the backend and pass in the wrong symbol.
What do you think?
Cheers,
Miles
The text was updated successfully, but these errors were encountered: