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

In-place quadgk #12

Closed
ChrisRackauckas opened this issue Jan 17, 2018 · 7 comments · Fixed by #45
Closed

In-place quadgk #12

ChrisRackauckas opened this issue Jan 17, 2018 · 7 comments · Fixed by #45

Comments

@ChrisRackauckas
Copy link
Member

While this package allows evaluation of integrands which return arrays, it's not efficient for the integrand to be an allocating f(t). Instead, quadgk should support a non-allocating form f!(out,t) for this case, for this version of the function do minimal internal allocation by caching necessary arrays and doing everything in-place.

@stevengj
Copy link
Member

stevengj commented Jan 17, 2018

Sounds good to me. I'm not entirely sure of the API for passing f!, though. quadgk!, since presumably you'd also want to pass a pre-allocated result array?

Some tricky refactoring might be required to keep the current generality (since not all types support things like .+=) while allowing in-place operations with preallocated arrays where possible.

One approach would be to internally wrap f! in an InplaceIntegrand functor type and dispatch on that, so that we can pass it through do_quadgk etcetera, and then add various methods foo(f, ...) which use the current behavior for generic f and an optimized in-place behavior for f::InplaceIntegrand.

@ChrisRackauckas
Copy link
Member Author

I think it would be quadgk! with a pre-allocated result array. I think to keep generality, the in-place one uses in-place operations and leaves the other one alone. quadgk would then be good for scalars and static arrays (so no broadcasting internally), while quadgk! would be more optimized for types which mutate and broadcast. Broadcast is almost a superset of indexable since GPUArrays and things like that are types which broadcast well but don't really index (they now have a fallback to avoid).

I would also like to have options to pass in the work vectors, much like it's done in the differentiation libraries, so I can avoid it allocating those each time. Or some kind of config/plan type that is made once and re-use.

@stevengj
Copy link
Member

I think to keep generality, the in-place one uses in-place operations and leaves the other one alone.

Yes, but it would be good to refactor so that they can share as much code as possible. I'd hate to see the entire current implementation copy-pasted and then modified.

@dominikkiese
Copy link

Has there been any progress on this? Came over this issue when using quadgk in multithreaded loops on Intel KNL cpus, as the allocation made by the integrand severly limits proper scaling.

@stevengj
Copy link
Member

No, no one has worked on this yet.

@stevengj
Copy link
Member

Note that for integrating small vector integrands, you can use SVector from StaticArrays.jl and it will be fast. See also this discussion on discourse.

@stevengj
Copy link
Member

stevengj commented Jun 19, 2020

I just pushed a PR to address this issue.

Note that it still cannot operate entirely in-place, because the QuadGK algorithm must dynamically allocate a heap data structure containing integration results on subintervals determined from error estimation.

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.

3 participants