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

restrict FastBroadcast to 0.3 and delete the extension #425

Merged

Conversation

oscardssmith
Copy link
Contributor

Surprisingly enough, with the new FastBroadcast version, everything just seems to work out of the box (and the extension was broken)

@ChrisRackauckas ChrisRackauckas merged commit 686259b into SciML:master Jun 13, 2024
5 checks passed
@oscardssmith oscardssmith deleted the os/restrict-to-FastBroadcast0.3 branch June 13, 2024 15:57
@isaacsas
Copy link
Member

isaacsas commented Jun 13, 2024

Everything just worked before the extension too, but without the extension it was falling back to very slow default versions. The extension was added because the performance was terrible with these fallbacks.

@isaacsas
Copy link
Member

What performance impact does dropping the extension have on systems with VariableRateJumps?

@isaacsas
Copy link
Member

#335

It would be unifortunate if the result of this is undoing all the work done to solve that issue

@oscardssmith
Copy link
Contributor Author

oscardssmith commented Jun 13, 2024

Looks like the performance is still good. Using the benchmark from the end of that issue, I see

julia> function test_single_dot(out, array)
            @inbounds  @. out = array + 1.0 * array + 1.2 * array
       end
test_single_dot (generic function with 1 method)

julia> @benchmark test_single_dot(bench_out_array, bench_in_array)
BenchmarkTools.Trial: 7308 samples with 1 evaluation.
 Range (min … max):  600.089 μs …  1.456 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     651.551 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   676.034 μs ± 80.437 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

    ▄█▅▂                                                        
  ▂▆█████▇▇▇▆▆▅▅▄▄▃▃▃▃▃▂▂▂▂▂▂▂▂▂▁▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  600 μs          Histogram: frequency by time         1.01 ms <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark test_single_dot(base_out_array, base_in_array)
BenchmarkTools.Trial: 7592 samples with 1 evaluation.
 Range (min … max):  604.956 μs …  1.456 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     632.637 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   650.833 μs ± 56.660 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

     ▄▇█▆▃                                                      
  ▁▄███████▆▅▄▄▄▄▄▄▄▄▃▃▂▃▂▂▂▂▂▂▂▂▂▂▂▁▁▁▂▁▁▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  605 μs          Histogram: frequency by time          829 μs <

 Memory estimate: 0 bytes, allocs estimate: 0.
 
 julia> function test_single_dot_dot(out, array)
            @inbounds  @.. out = array + 1.0 * array + 1.2 * array
       end
test_single_dot (generic function with 1 method)

julia> @benchmark test_single_dot_dot(base_out_array, base_in_array)
BenchmarkTools.Trial: 7130 samples with 1 evaluation.
 Range (min … max):  576.155 μs …   1.627 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     659.532 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   691.824 μs ± 119.064 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

   ▄█▆▄▄▄▅▄▄▃▁                                                   
  ▄████████████▇▆▆▅▄▃▃▃▃▂▂▂▂▂▂▂▂▁▁▂▁▁▁▁▁▁▁▁▁▁▁▂▁▁▂▂▁▂▁▂▁▁▁▁▁▁▁▁ ▃
  576 μs           Histogram: frequency by time         1.18 ms <

 Memory estimate: 0 bytes, allocs estimate: 0.

julia> @benchmark test_single_dot_dot(bench_out_array, bench_in_array)
BenchmarkTools.Trial: 7530 samples with 1 evaluation.
 Range (min … max):  598.814 μs …  1.321 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     640.346 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   655.685 μs ± 50.883 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

    ▁▆██▅▃▁                                                     
  ▁▄███████▇▆▆▆▆▇▆▆▆▅▅▅▅▄▄▄▃▃▃▂▃▂▂▂▂▂▂▂▁▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▃
  599 μs          Histogram: frequency by time          846 μs <

 Memory estimate: 0 bytes, allocs estimate: 0.

@isaacsas
Copy link
Member

Ok, thanks for checking.

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.

3 participants