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

Fix changebasis_I #323

Merged
merged 42 commits into from
Sep 18, 2023
Merged

Fix changebasis_I #323

merged 42 commits into from
Sep 18, 2023

Conversation

hyrodium
Copy link
Owner

This PR fixes #177 finally!

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Merging #323 (652a2e7) into main (b0221e6) will increase coverage by 0.08%.
The diff coverage is 91.71%.

@@            Coverage Diff             @@
##             main     #323      +/-   ##
==========================================
+ Coverage   96.61%   96.69%   +0.08%     
==========================================
  Files          13       13              
  Lines        1359     1513     +154     
==========================================
+ Hits         1313     1463     +150     
- Misses         46       50       +4     
Files Changed Coverage Δ
src/_BSplineSpace.jl 98.02% <ø> (+6.57%) ⬆️
src/_ChangeBasis.jl 92.98% <91.30%> (-1.36%) ⬇️
src/_BSplineBasis.jl 99.28% <94.73%> (-0.72%) ⬇️
src/_KnotVector.jl 95.36% <100.00%> (+0.03%) ⬆️

@hyrodium
Copy link
Owner Author

Finally! 🎉

julia> using BasicBSpline

julia> p = 3
3

julia> P1 = BSplineSpace{p}(KnotVector(1:8))
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 2, 3, 4, 5, 6, 7, 8]))

julia> P2 = BSplineSpace{p}(KnotVector([2,3,3,4,5,6,7,8]))
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([2, 3, 3, 4, 5, 6, 7, 8]))

julia> changebasis(P1, P2)
4×4 SparseArrays.SparseMatrixCSC{Float64, Int32} with 5 stored entries:
 0.666667            
 0.333333  1.0        
              1.0    
                  1.0

julia> P = BSplineSpace{p}(KnotVector(1:10))
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]))

julia> P′ = BSplineSpace{p}(KnotVector([1,2,3,4,4.5,5,6,7,8.1,9.1,10.5]))
BSplineSpace{3, Float64, KnotVector{Float64}}(KnotVector([1.0, 2.0, 3.0, 4.0, 4.5, 5.0, 6.0, 7.0, 8.1, 9.1, 10.5]))

julia> changebasis(P,P′)
6×7 SparseArrays.SparseMatrixCSC{Float64, Int32} with 13 stored entries:
 1.0  0.166667                                 
     0.833333  0.5                             
              0.5  0.833333                    
                  0.166667  1.0  -0.0333333   0.0183333
                                1.03333    -0.103333
                                           1.085

@hyrodium
Copy link
Owner Author

Okay, there are a lot of things to improve changebasis_I than I thought. There are still some TODOs, but I will merge this PR tomorrow, after adding more TODO comments. I hope I can release v0.10.0 in this year:pray:

@hyrodium hyrodium marked this pull request as ready for review September 17, 2023 18:05
@hyrodium
Copy link
Owner Author

Performance improvements on changebasis_R🎉

Before this PR

julia> using BasicBSpline, BenchmarkTools

julia> k1 = knotvector"4 1 2 32 1111"
KnotVector([1, 1, 1, 1, 3, 5, 5, 7, 7, 7, 8, 8, 10, 11, 12, 13])

julia> k2 = knotvector"4321214311111"
KnotVector([1, 1, 1, 1, 2, 2, 2, 3, 3, 4, 5, 5, 6, 7, 7, 7, 7, 8, 8, 8, 9, 10, 11, 12, 13])

julia> P1 = BSplineSpace{3}(k1)
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 1, 1, 1, 3, 5, 5, 7, 7, 7, 8, 8, 10, 11, 12, 13]))

julia> P2 = BSplineSpace{3}(k2)
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 1, 1, 1, 2, 2, 2, 3, 3, 4, 5, 5, 6, 7, 7, 7, 7, 8, 8, 8, 9, 10, 11, 12, 13]))

julia> P1  P2
true

julia> changebasis(P1, P2)
12×21 SparseArrays.SparseMatrixCSC{Float64, Int32} with 42 stored entries:
⎡⠙⢿⣶⣦⣀⠀⠀⠀⠀⠀⠀⎤
⎢⠀⠀⠀⠀⠉⠳⠤⡀⠀⠀⠀⎥
⎣⠀⠀⠀⠀⠀⠀⠀⠈⠛⢦⡀⎦

julia> @benchmark changebasis(P1, P2)
BenchmarkTools.Trial: 10000 samples with 3 evaluations.
 Range (min  max):   8.777 μs  712.151 μs  ┊ GC (min  max): 0.00%  95.12%
 Time  (median):      9.545 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   10.426 μs ±  16.585 μs  ┊ GC (mean ± σ):  4.05% ±  2.51%

    ▃▅▇███▇▆▅▄▃▁    ▁ ▁▁▁▁▁▂▂▃▂▃▂▂▂▂▁▁▁                        ▂
  ▆███████████████▇███████████████████████▇█▇▇▇▆▆▇▆▆▆▇▇▆▇▆▇▆▆▅ █
  8.78 μs       Histogram: log(frequency) by time      14.9 μs <

 Memory estimate: 11.09 KiB, allocs estimate: 76.

After this PR

julia> using BasicBSpline, BenchmarkTools

julia> k1 = knotvector"4 1 2 32 1111"
KnotVector([1, 1, 1, 1, 3, 5, 5, 7, 7, 7, 8, 8, 10, 11, 12, 13])

julia> k2 = knotvector"4321214311111"
KnotVector([1, 1, 1, 1, 2, 2, 2, 3, 3, 4, 5, 5, 6, 7, 7, 7, 7, 8, 8, 8, 9, 10, 11, 12, 13])

julia> P1 = BSplineSpace{3}(k1)
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 1, 1, 1, 3, 5, 5, 7, 7, 7, 8, 8, 10, 11, 12, 13]))

julia> P2 = BSplineSpace{3}(k2)
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 1, 1, 1, 2, 2, 2, 3, 3, 4, 5, 5, 6, 7, 7, 7, 7, 8, 8, 8, 9, 10, 11, 12, 13]))

julia> P1  P2
true

julia> changebasis(P1, P2)
12×21 SparseArrays.SparseMatrixCSC{Float64, Int32} with 42 stored entries:
⎡⠙⢿⣶⣦⣀⠀⠀⠀⠀⠀⠀⎤
⎢⠀⠀⠀⠀⠉⠳⠤⡀⠀⠀⠀⎥
⎣⠀⠀⠀⠀⠀⠀⠀⠈⠛⢦⡀⎦

julia> @benchmark changebasis(P1, P2)
BenchmarkTools.Trial: 10000 samples with 6 evaluations.
 Range (min  max):  5.891 μs  355.995 μs  ┊ GC (min  max): 0.00%  94.82%
 Time  (median):     6.509 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   7.387 μs ±  11.802 μs  ┊ GC (mean ± σ):  5.75% ±  3.54%

   ▁▄▇███▇▆▄▃▂▁▂▃▄▄▄▃▃▂▁    ▁  ▁▁▁▁ ▁     ▁                   ▂
  ▇█████████████████████████████████████████████▇▇▇▇▇▆▆▅▆▆▆▆▅ █
  5.89 μs      Histogram: log(frequency) by time      11.4 μs <

 Memory estimate: 11.00 KiB, allocs estimate: 70.

@hyrodium
Copy link
Owner Author

hyrodium commented Sep 18, 2023

The performance of chagangebasis_I is getting much worse. This will be fixed in the future PRs.

Before this PR

julia> using BasicBSpline, BenchmarkTools

julia> p = 3
3

julia> P = BSplineSpace{p}(KnotVector(1:10))
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]))

julia> P′ = BSplineSpace{p}(KnotVector([1,2,3,4,4.5,5,6,7,8.1,9.1,10.5]))
BSplineSpace{3, Float64, KnotVector{Float64}}(KnotVector([1.0, 2.0, 3.0, 4.0, 4.5, 5.0, 6.0, 7.0, 8.1, 9.1, 10.5]))

julia> changebasis(P,P′)
6×7 SparseArrays.SparseMatrixCSC{Float64, Int32} with 16 stored entries:
 1.0          0.166667                                 
 8.88178e-16  0.833333  0.5                             
 4.44089e-16  0.0       0.5  0.833333                    
                          0.166667  1.0  -0.0333333   0.0183333
                                        1.03333    -0.103333
                                                   1.085

julia> @benchmark changebasis(P,P′)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min  max):   9.387 μs   3.547 ms  ┊ GC (min  max): 0.00%  97.65%
 Time  (median):     10.710 μs              ┊ GC (median):    0.00%
 Time  (mean ± σ):   12.354 μs ± 59.941 μs  ┊ GC (mean ± σ):  8.23% ±  1.69%

     ▁▅▇█▅▅▂▁                                                  
  ▁▂▅████████▇▅▄▃▃▃▃▃▃▃▃▂▂▂▂▂▂▂▂▂▂▂▂▁▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  9.39 μs         Histogram: frequency by time        18.7 μs <

 Memory estimate: 13.78 KiB, allocs estimate: 153.

After this PR

julia> using BasicBSpline, BenchmarkTools

julia> p = 3
3

julia> P = BSplineSpace{p}(KnotVector(1:10))
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]))

julia> P′ = BSplineSpace{p}(KnotVector([1,2,3,4,4.5,5,6,7,8.1,9.1,10.5]))
BSplineSpace{3, Float64, KnotVector{Float64}}(KnotVector([1.0, 2.0, 3.0, 4.0, 4.5, 5.0, 6.0, 7.0, 8.1, 9.1, 10.5]))

julia> changebasis(P,P′)
6×7 SparseArrays.SparseMatrixCSC{Float64, Int32} with 13 stored entries:
 1.0  0.166667                                 
     0.833333  0.5                             
              0.5  0.833333                    
                  0.166667  1.0  -0.0333333   0.0183333
                                1.03333    -0.103333
                                           1.085

julia> @benchmark changebasis(P,P′)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min  max):   87.757 μs    6.008 ms  ┊ GC (min  max):  0.00%  94.09%
 Time  (median):      93.898 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   109.354 μs ± 225.588 μs  ┊ GC (mean ± σ):  11.39% ±  5.40%

   ▁▅▇█▇▆▅▃▃▂▂▁▁▂▂▃▃▂▁▁▁▁                                       ▂
  ▅█████████████████████████▇▇▇▆▆▄▆▅▃▄▅▄▁▃▃▄▄▃▅▄▁▄▃▄▃▃▃▄▁▁▄▁▁▁▅ █
  87.8 μs       Histogram: log(frequency) by time        162 μs <

 Memory estimate: 145.64 KiB, allocs estimate: 1663.

@hyrodium hyrodium merged commit 32206ca into main Sep 18, 2023
10 of 12 checks passed
@hyrodium hyrodium deleted the feature/changebasis_I branch October 28, 2023 04:54
@hyrodium hyrodium mentioned this pull request Nov 12, 2023
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.

changebasis sometimes returns incorrect nonzero element
1 participant