-
Notifications
You must be signed in to change notification settings - Fork 25
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
Precompute grids and coeffs #61
Conversation
From stacking this ontop of #60:
With #60 alone:
interesting that it seems to make the |
1 similar comment
From stacking this ontop of #60:
With #60 alone:
interesting that it seems to make the |
Instead of precomputing you could memoize. Avoids having to determine upfront what exact combinations should be precomputed. Taking the cost once per session is probably ok. |
@eval begin | ||
# Compatibility layer over the "old" API | ||
function $fdmf(p::Integer, q::Integer; adapt=1, kwargs...) | ||
_dep_kwarg(kwargs) | ||
return $D(p, q; adapt=adapt, kwargs...) | ||
end | ||
|
||
# precompute a bunch of grids and coefs | ||
const $precomputes = ntuple($num_precomputed) do p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be defined in a constant?
Or should it be declared outside the eval, and then inlined in as a local literal
and thus should constant fold
Yeah, good idea. I think I might have looked into it, and struggled to workout how to do it without getting allocations. But I can't recall if I did, so I should check again |
No point with #73 |
grids and coeffs are always the same for standard grids for given
p
andq
.Precomputing them helps decrease the cost assoicated with
adapt
in the gradients.This also fixes up some of the type instability with the central gradient, during adapt.
Current
New with Precomputed coeffs