Skip to content

Conversation

@albertomercurio
Copy link
Contributor

The current implementation of FunctionOperator requires a Dict{Symbol,Any} to handle the keyword arguments. This leads to type instabilities and performance decreases, as in the following simple code:

using LinearAlgebra
using BenchmarkTools
using SciMLOperators

T = ComplexF64
N = 10

u = rand(T, N)
du = similar(u)
p = (A=rand(T, N, N),)

op = FunctionOperator((du, u, p, t) -> mul!(du, p.A, u), u, p = p, T=T, isinplace=Val(true), outofplace=Val(true), has_mul5=Val(true), isconstant=true)

op(du, u, p, 0.1)

@benchmark $op($du, $u, $p, 0.1)

master branch

BenchmarkTools.Trial: 10000 samples with 53 evaluations.
 Range (min … max):  864.434 ns … 104.262 μs  ┊ GC (min … max): 0.00% … 98.52%
 Time  (median):     947.340 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   996.750 ns ±   1.294 μs  ┊ GC (mean ± σ):  2.57% ±  2.17%

           ▃█                                                    
  ▂▂▃▅▄▃▃▃▅███▄▃▃▃▃▃▃▃▃▄▅▄▄▃▂▂▂▂▂▂▂▂▂▂▂▁▂▁▂▂▂▂▂▂▂▂▂▁▂▂▂▂▂▂▂▂▂▂▂ ▃
  864 ns           Histogram: frequency by time         1.34 μs <

 Memory estimate: 560 bytes, allocs estimate: 7.

This PR branch

BenchmarkTools.Trial: 10000 samples with 989 evaluations.
 Range (min … max):  46.024 ns … 64.056 ns  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     48.916 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   48.858 ns ±  1.040 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

   ▃▄▄▂  ▁▁▁▂▂▂▂▁    ▁▁▁▁▁▁▁▁▁ ▁▂▃▆▇█▇▆▅▄▅▅▅▅▅▅▄▄▄▁▁▁▁▁▁▂▂▁▁  ▃
  ▇██████████████████████████████████████████████████████████ █
  46 ns        Histogram: log(frequency) by time      50.9 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

To manage to fix this issue, I used NamedTuple instead of Dict{Symbol, Any}. For this reason, the additional kwargs are required when accepted_kwargs is provided, in order to initialize the types inside the NamedTuple. In principle, the accepted_kwargs can be directly obtained from kwargs keys, but I left it for consistency with the other functions.

Furthermore, I've extended the methods of the get_filtered_kwargs, also supporting Val types for accepted_kwargs. In this way, the filtering is performed at compile time, otherwise the type of the NamedTuple cannot be inferred. Thus, all the other functions supporting accepted_kwargs would also support Val types, improving performance.

@albertomercurio albertomercurio changed the title Remove Dict{Symbol, Any} for type stability Remove Dict{Symbol, Any} for type stability in FunctionOperator Oct 19, 2024
@albertomercurio
Copy link
Contributor Author

I've also made the mul! function for the IdentityOperator, because the following example was allocating

T = Float64
N = 10

u = rand(T, N)
du = similar(u)

A = MatrixOperator(rand(T, N, N))

op = T(0.1) * A + T(0.2) * IdentityOperator(N)
# op = T(0.2) * IdentityOperator(N)
# op = T(0.1) * A

@benchmark mul!($du, $op, $u)

Master branch

BenchmarkTools.Trial: 10000 samples with 982 evaluations.
 Range (min … max):  57.484 ns …  14.506 μs  ┊ GC (min … max): 0.00% … 99.22%
 Time  (median):     67.768 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   79.566 ns ± 198.549 ns  ┊ GC (mean ± σ):  6.26% ±  2.77%

           ▁▃▅█▇▂                                               
  ▂▁▂▂▃▃▅▇███████▆▄▃▃▂▂▂▂▃▂▂▃▄▅▅▄▃▃▂▂▂▂▂▃▃▄▄▄▄▅▇██▇▄▃▃▂▂▂▂▂▂▂▂ ▃
  57.5 ns         Histogram: frequency by time          101 ns <

 Memory estimate: 112 bytes, allocs estimate: 3.

This PR branch

BenchmarkTools.Trial: 10000 samples with 994 evaluations.
 Range (min … max):  31.203 ns … 42.741 ns  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     33.894 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   33.676 ns ±  0.924 ns  ┊ GC (mean ± σ):  0.00% ± 0.00%

                                  ▁▃▅█▇▄▂▁                     
  ▁▁▁▂▁▂▂▂▃▃▅▅▅▅▅▄▄▃▂▂▂▂▂▂▂▄▄▅▅▅▇▇████████▆▅▄▃▃▃▃▃▃▂▂▂▂▂▂▁▁▁▁ ▃
  31.2 ns         Histogram: frequency by time        35.8 ns <

 Memory estimate: 0 bytes, allocs estimate: 0.

@ChrisRackauckas ChrisRackauckas merged commit c613fe3 into SciML:master Oct 21, 2024
7 of 12 checks passed
@ChrisRackauckas
Copy link
Member

this repo needs quite a bit of work in order to v1.0. Seems like you're starting to get into it. If you're interested in helping, maybe we can find a time to chat. The core issue is that a lot of the operators do f(u,p,t)*u when it needs to generally be f(u,p,t)*v

@albertomercurio
Copy link
Contributor Author

Yes, I'm starting to use it due to its dependence of my package QuantumToolbox.jl. I don't think I have so much time for directly helping this repo, but for sure I will make other PRs when I will need it. Btw, I'm happy to have a chat and discuss about this.

ChrisRackauckas-Claude pushed a commit to ChrisRackauckas-Claude/SciMLOperators.jl that referenced this pull request Oct 9, 2025
This commit resolves issue SciML#312 by converting accepted_kwargs tuples
to Val types for compile-time kwarg filtering, eliminating runtime
allocations.

Changes:
- Modified preprocess_update_func to automatically convert tuple
  accepted_kwargs to Val types
- Updated get_filtered_kwargs to handle missing keys gracefully
  when using Val types

This follows the same pattern introduced in PR SciML#255 for FunctionOperator
but extends it to all operators (MatrixOperator, AffineOperator, etc.)
by fixing it at the preprocess_update_func level.

Before: 9 allocations (272 bytes) when using accepted_kwargs
After: 0 allocations

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
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 this pull request may close these issues.

2 participants