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

Support inv on diagonal matrices from unitranges #177

Merged
merged 3 commits into from
Jun 17, 2024

Conversation

DanielVandH
Copy link
Contributor

Fixes #176

@jishnub
Copy link
Member

jishnub commented Jun 17, 2024

Won't this change the behavior from throwing a SingularException to returning an Inf if one of the values is zero?

@DanielVandH
Copy link
Contributor Author

Technically yes, although this inv never worked in the first place for nonsingular matrices. Similar methods for inv don't throw, e.g.

julia> A = Diagonal((0:∞) .+ (0:∞)) # force non-infrange type
ℵ₀×ℵ₀ Diagonal{Int64, LazyArrays.BroadcastVector{Int64, typeof(+), Tuple{InfiniteArrays.InfUnitRange{Int64}, InfiniteArrays.InfUnitRange{Int64}}}} with indices OneToInf()×OneToInf():
 0                                        
   2                                    
     4                                  
       6                                
         8                              
           10                             
              12                        
                 14                     
                    16                  
                       18               
                          20              
                             22         
                                24      
                                                     

julia> inv(A)
ℵ₀×ℵ₀ Diagonal{Float64, LazyArrays.BroadcastVector{Float64, typeof(inv), Tuple{LazyArrays.BroadcastVector{Int64, typeof(+), Tuple{InfiniteArrays.InfUnitRange{Int64}, InfiniteArrays.InfUnitRange{Int64}}}}}} with indices OneToInf()×OneToInf():
 Inf                                         
      0.5                           
          0.25                      
               0.166667             
                        0.125       
                              0.1            
                                  0.0833333
                                   
                                   
                                   
                                            
                                   
                                   
                                                 

@jishnub
Copy link
Member

jishnub commented Jun 17, 2024

Should a change like this go to LinearAlgebra? That way, we won't need to special case this for other infinite array types.

@DanielVandH
Copy link
Contributor Author

DanielVandH commented Jun 17, 2024

I wasn't too sure, the method in LinearAlgebra does just loop over each element and check iszero for each D.diag[i] which is probably more effective than my inv.(D.diag) for finite arrays. What if I instead did

function inv(D::Diagonal{T, <:InfRanges}) where {T} 
    d = D.diag 
    idx = findfirst(==(zero(T)), d) # iszero is broken for infranges
    isnothing(idx) || throw(SingularException(idx))
    return Diagonal(inv.(d))
end

I do agree that having to special case is a bit annoying but I don't think such a change would get through to LinearAlgebra

@jishnub
Copy link
Member

jishnub commented Jun 17, 2024

In any case, we can add it here, as we would still need it on older versions of Julia

Copy link

codecov bot commented Jun 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.97%. Comparing base (d6eaad4) to head (603baf5).
Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #177      +/-   ##
==========================================
- Coverage   86.00%   84.97%   -1.03%     
==========================================
  Files           6        6              
  Lines         743      752       +9     
==========================================
  Hits          639      639              
- Misses        104      113       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dlfivefifty
Copy link
Member

Test failure seems spurious

@dlfivefifty dlfivefifty merged commit ce6da6b into JuliaArrays:master Jun 17, 2024
8 of 9 checks passed
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.

Can't invert Diagonal{Int, InfUnitRange}
3 participants