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

Add internal _degeneratize_R and _degeneratize_I #342

Merged
merged 12 commits into from
Nov 11, 2023

Conversation

hyrodium
Copy link
Owner

@hyrodium hyrodium commented Nov 11, 2023

This PR fixes #325.

Before this PR

julia> using BasicBSpline

julia> Pd1 = BSplineSpace{2}(knotvector" 21 3")
BSplineSpace{2, Int64, KnotVector{Int64}}(KnotVector([2, 2, 3, 5, 5, 5]))

julia> Pd2 = BSplineSpace{3}(knotvector"212 4")
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 1, 2, 3, 3, 5, 5, 5, 5]))

julia> BasicBSpline.changebasis_I(Pd1, Pd2)
ERROR: MethodError: no method matching isless(::Nothing, ::Nothing)

Closest candidates are:
  isless(::Any, ::Missing)
   @ Base missing.jl:88
  isless(::Missing, ::Any)
   @ Base missing.jl:87

Stacktrace:
 [1] <(x::Nothing, y::Nothing)
   @ Base ./operators.jl:343
 [2] <=(x::Nothing, y::Nothing)
   @ Base ./operators.jl:392
 [3] >=(x::Nothing, y::Nothing)
   @ Base ./operators.jl:416
 [4] (::Colon)(start::Nothing, stop::Nothing)
   @ Base ./range.jl:7
 [5] _find_j_range_I(P::BSplineSpace{2, Int64, KnotVector{Int64}}, P′::BSplineSpace{3, Int64, KnotVector{Int64}}, i::Int64, j_begin::Int64, j_end::Int64)
   @ BasicBSpline ~/.julia/dev/BasicBSpline/src/_ChangeBasis.jl:405
 [6] _changebasis_I(P::BSplineSpace{2, Int64, KnotVector{Int64}}, P′::BSplineSpace{3, Int64, KnotVector{Int64}})
   @ BasicBSpline ~/.julia/dev/BasicBSpline/src/_ChangeBasis.jl:527
 [7] changebasis_I(P::BSplineSpace{2, Int64, KnotVector{Int64}}, P′::BSplineSpace{3, Int64, KnotVector{Int64}})
   @ BasicBSpline ~/.julia/dev/BasicBSpline/src/_ChangeBasis.jl:350
 [8] top-level scope
   @ REPL[4]:1

After this PR

julia> using BasicBSpline

julia> Pd1 = BSplineSpace{2}(knotvector" 21 3")
BSplineSpace{2, Int64, KnotVector{Int64}}(KnotVector([2, 2, 3, 5, 5, 5]))

julia> Pd2 = BSplineSpace{3}(knotvector"212 4")
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 1, 2, 3, 3, 5, 5, 5, 5]))

julia> BasicBSpline.changebasis_I(Pd1, Pd2)
3×5 SparseArrays.SparseMatrixCSC{Float64, Int32} with 7 stored entries:
     0.888889  0.222222             
     0.111111  0.777778  0.666667    
                       0.333333  1.0

Copy link

codecov bot commented Nov 11, 2023

Codecov Report

Merging #342 (678a931) into main (474e85d) will decrease coverage by 0.46%.
The diff coverage is 77.14%.

@@            Coverage Diff             @@
##             main     #342      +/-   ##
==========================================
- Coverage   97.02%   96.56%   -0.46%     
==========================================
  Files          14       14              
  Lines        1547     1574      +27     
==========================================
+ Hits         1501     1520      +19     
- Misses         46       54       +8     
Files Coverage Δ
src/_BSplineBasis.jl 99.28% <100.00%> (ø)
src/_ChangeBasis.jl 94.08% <100.00%> (+0.10%) ⬆️
src/_BSplineSpace.jl 93.85% <60.00%> (-4.26%) ⬇️

@hyrodium hyrodium force-pushed the feature/internal_degeneratize branch from 7edc64c to fb04c55 Compare November 11, 2023 07:21
@hyrodium
Copy link
Owner Author

Another case:

julia> using BasicBSpline

julia> Pd1 = BSplineSpace{3}(knotvector"44 3  2 121 ")
BSplineSpace{3, Int64, KnotVector{Int64}}(KnotVector([1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 7, 7, 9, 10, 10, 11]))

julia> Pd2 = BSplineSpace{4}(knotvector"55 62 3 2121")
BSplineSpace{4, Int64, KnotVector{Int64}}(KnotVector([1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 4, 4, 4, 4, 4, 4, 5, 5, 7, 7, 7, 9, 9, 10, 11, 11, 12]))

julia> changebasis_I(Pd1, Pd2)
13×22 SparseArrays.SparseMatrixCSC{Float64, Int32} with 41 stored entries:
⎡⠙⢦⡀⠀⠀⠀⠀⠀⠀⠀⠀⎤
⎢⠀⠀⠈⠳⣄⢀⣀⡀⠀⠀⠀⎥
⎢⠀⠀⠀⠀⠀⠀⠙⠿⠶⣤⡄⎥
⎣⠀⠀⠀⠀⠀⠀⠀⠀⠀⠈⠁⎦

julia> Matrix(changebasis_I(Pd1, Pd2))
13×22 Matrix{Float64}:
 1.0  0.25  0.0  0.0   0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.75  0.5  0.0   0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.5  0.75  0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.25  1.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.0   0.0  1.0  0.25  0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.75  0.5  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.0   0.5  0.75  0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.0   0.0  0.25  1.0  0.0  1.0  0.75  0.555556   0.111111   0.0       0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.25  0.388889   0.555556   0.222222  0.0  0.0  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0555556  0.316667   0.644444  0.7  0.1  0.0       0.0         0.0        0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0166667  0.133333  0.3  0.9  0.666667  0.0833333  -0.0277778  0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.333333  0.75       -0.0277778  0.0
 0.0  0.0   0.0  0.0   0.0  0.0  0.0   0.0  0.0   0.0  0.0  0.0  0.0   0.0        0.0        0.0       0.0  0.0  0.0       0.166667    1.05556    0.0

@hyrodium hyrodium merged commit 9acfde8 into main Nov 11, 2023
10 of 12 checks passed
@hyrodium
Copy link
Owner Author

Oh, _degeneratize_R and _degeneratize_I should be _nondegeneratize_R and _nondegeneratize_I.

@hyrodium hyrodium deleted the feature/internal_degeneratize branch November 12, 2023 12:30
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_I sometimes returns a matrix with NaN
1 participant