Closed adrhill closed 3 months ago
I think it stands for autodiff but I might be wrong
It just felt nice when we named these. Also dropping them would make it identical to the module names hence not a valid identifier
It might be too late to rename and I realize the issue is closed, but I spent a day or two trying to figure out what AutoZygote()
was actually doing. I didn't realize it was just a naming convention. Might I suggest to
AutoZygote
to UseZygote()
Tbh I find it quite logical so I am not sure, I interpret it as - "Automatically use Zygote". Naming can be quite subjective so unless there's a technical reason for not liking this I don't see this conversation reaching a conclusion 😅
I interpret it as - "Automatically use Zygote".
As suggested on Slack by @mcabbott, adding this to the docstrings and mentioning that these types come from ADTypes (not everyone is aware of parentmodule
) is probably enough to avoid the issue.
It just felt nice when we named these. Also dropping them would make it identical to the module names hence not a valid identifier
That's the big reason. You can't just call it Zygote()
otherwise lots of stuff happens, so what do you append it with? Auto
for automatic differentiation or automatic choice made sense. We can bikeshed names if we want, ultimately it's not a big deal if there's a deprecation path for it. But if you do the change then I expect that you update all downstream code, SciML and beyond, because it's part of the policy to follow through with breaking changes.
It looks like all exported types have
Auto
in their names. Why "Auto" and why not drop this prefix? I feel like this implies the existence of some "Manual" AD backends.https://github.com/SciML/ADTypes.jl/blob/59ded48442fb3806f87fd65fdeb40a681af68b2e/src/ADTypes.jl#L225-L239