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

StaticArrays support #768

Merged
merged 2 commits into from
Aug 9, 2023

Conversation

amontoison
Copy link
Member

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

Package name latest stable
CaNNOLeS.jl
DCISolver.jl
FletcherPenaltySolver.jl
JSOSolvers.jl
LLSModels.jl
LinearSolve.jl
Percival.jl
RipQP.jl

@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (88f518b) 98.26% compared to head (84990dd) 98.26%.

❗ Current head 84990dd differs from pull request most recent head a1bfdda. Consider uploading reports for the commit a1bfdda to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #768   +/-   ##
=======================================
  Coverage   98.26%   98.26%           
=======================================
  Files          39       39           
  Lines        7105     7105           
=======================================
  Hits         6982     6982           
  Misses        123      123           
Files Changed Coverage Δ
src/krylov_utils.jl 91.37% <0.00%> (ø)

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

@@ -210,9 +210,9 @@ function ktypeof(v::S) where S <: DenseVector
end

function ktypeof(v::S) where S <: AbstractVector
if S.name.name == :Zeros || S.name.name == :Ones
if S.name.name == :Zeros || S.name.name == :Ones || S.name.name == :SArray || S.name.name == :MArray || S.name.name == :SizedArray
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think :FieldArray is also a valid subtype of StaticVector

julia> subtypes(StaticVector)
5-element Vector{Any}:
 FieldArray{Tuple{N}, T, 1} where {N, T}
 MArray{Tuple{N}, T, 1} where {N, T}
 SArray{Tuple{N}, T, 1} where {N, T}
 SizedArray{Tuple{N}, T, 1, M} where {N, T, M}
 StaticArrays.SUnitRange

Copy link

@gdalle gdalle Aug 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if we could add :ComponentArray that would solve all my immediate problems 🙏

@gdalle
Copy link

gdalle commented Aug 8, 2023

This works for my use case, thanks for the rapid fix! I'm gonna try to demo a more principled version over on #767, but if you can merge this in the meantime that would be very helpful

using LinearAlgebra, SparseArrays, Test
using Krylov, StaticArrays

@testset "StaticArrays" begin
Copy link

@gdalle gdalle Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to also test other array formats like FillArrays? Test dependencies are basically free

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I will do that with another PR.

@gdalle
Copy link

gdalle commented Aug 9, 2023

Given #769 I really think the right fix is #767 instead of editing this single line forever with manual type names.

Plus, if someone happens to redefine Ones outside of FillArrays.jl, the current code has no way to tell.

@amontoison amontoison merged commit 165db17 into JuliaSmoothOptimizers:main Aug 9, 2023
28 of 32 checks passed
@amontoison amontoison deleted the static-arrays branch August 9, 2023 17:34
@gdalle
Copy link

gdalle commented Aug 9, 2023

my hero @amontoison, thank you

@gdalle gdalle mentioned this pull request Aug 9, 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.

2 participants