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

remove unused import of static_length #68

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

ranocha
Copy link
Member

@ranocha ranocha commented Feb 24, 2023

I couldn't find any reference to static_length in StrideArrays.jl. Thus, I removed it to prepare for the update to ArrayInterface.jl v7.

I couldn't find any reference to `static_length` in StrideArrays.jl. Thus, I removed it to prepare for the update to ArrayInterface.jl v7.
@chriselrod
Copy link
Member

Okay, but do note that every instance of length are likely to become static_length following the upgrade.

@chriselrod
Copy link
Member

Also, it may be worth moving most of this package into PkgExtensions for StrideArraysCore.

@ranocha
Copy link
Member Author

ranocha commented Feb 25, 2023

Yeah, I agree. I just wanted to start with something simple to see whether CI passes. Looks like there are two issues

Have you seen that before?

@chriselrod
Copy link
Member

That this type priates StrideArraysCore is expected.
Why vcat no longer returns static sizes will need investigating.

@ranocha
Copy link
Member Author

ranocha commented Mar 12, 2023

Why vcat no longer returns static sizes will need investigating.

Bisected to the update of StrideArraysCore.jl v0.4.6 -> v0.4.7 using the following code

julia> using Pkg; Pkg.activate(temp = true); Pkg.add([
           PackageSpec(name = "StrideArrays", version = "0.1.23"),
           PackageSpec(name = "StrideArraysCore", version = "0.4.7"),
       ]); using StrideArrays

julia> let
       A = @StrideArray rand($(5 << 1), $(1 << 3))
       A_u = view(A, StaticInt(1):StaticInt(6), :)
       A_l = view(A, StaticInt(7):StaticInt(10), :)
       StrideArrays.size(A) === StrideArrays.size(vcat(A_u, A_l))
       end
false # false with v0.4.7, true with v0.4.6

The diff is JuliaSIMD/StrideArraysCore.jl@v0.4.6...v0.4.7

Does this already tell you what's going wrong, @chriselrod?

@ranocha
Copy link
Member Author

ranocha commented Mar 12, 2023

That this type priates StrideArraysCore is expected.

I disabled the piracy tests. Since this requires Aqua.jl v0.6, I added compat bounds to test/Project.toml and let CompatHelper track them.

@ranocha
Copy link
Member Author

ranocha commented Mar 12, 2023

Why vcat no longer returns static sizes will need investigating.

Bisected to the update of StrideArraysCore.jl v0.4.6 -> v0.4.7 using the following code

julia> using Pkg; Pkg.activate(temp = true); Pkg.add([
           PackageSpec(name = "StrideArrays", version = "0.1.23"),
           PackageSpec(name = "StrideArraysCore", version = "0.4.7"),
       ]); using StrideArrays

julia> let
       A = @StrideArray rand($(5 << 1), $(1 << 3))
       A_u = view(A, StaticInt(1):StaticInt(6), :)
       A_l = view(A, StaticInt(7):StaticInt(10), :)
       StrideArrays.size(A) === StrideArrays.size(vcat(A_u, A_l))
       end
false # false with v0.4.7, true with v0.4.6

The diff is JuliaSIMD/[email protected]

It looks like the views are already broken: StrideArraysCore.jl v0.4.6 (calling a method in StrideArraysCore.jl) gives

julia> A = @StrideArray rand($(5 << 1), $(1 << 3)); A_u = view(A, StaticInt(1):StaticInt(6), :)
6×8 StrideArray{Float64, 2, (1, 2), Tuple{StaticInt{6}, StaticInt{8}}, Tuple{Nothing, StrideArraysCore.StrideReset{StaticInt{10}}}, Tuple{StaticInt{1}, StaticInt{1}}, StrideArraysCore.StaticStrideArray{Float64, 2, (1, 2), Tuple{StaticInt{10}, StaticInt{8}}, Tuple{Nothing, Nothing}, Tuple{StaticInt{1}, StaticInt{1}}, 80}} with indices static(1):static(6)×static(1):static(8):
 0.191039  0.574006   0.290582   0.175618  0.178092   0.196963  0.154767  0.541234
 0.392801  0.599062   0.735296   0.482211  0.151804   0.807586  0.16376   0.118286
 0.738249  0.913247   0.0873968  0.452708  0.473348   0.745417  0.758472  0.778555
 0.749282  0.423333   0.597436   0.805952  0.747627   0.392767  0.287661  0.687246
 0.982385  0.913687   0.759203   0.987565  0.47629    0.21957   0.869044  0.905959
 0.308999  0.0626124  0.609769   0.958194  0.0620535  0.890362  0.411321  0.341948

while StrideArraysCore.jl v0.4.7 (calling a method from Base) yields

julia> A = @StrideArray rand($(5 << 1), $(1 << 3)); A_u = view(A, StaticInt(1):StaticInt(6), :)
6×8 view(::StrideArraysCore.StaticStrideArray{Float64, 2, (1, 2), Tuple{StaticInt{10}, StaticInt{8}}, Tuple{Nothing, Nothing}, Tuple{StaticInt{1}, StaticInt{1}}, 80}, static(1):static(6), :) with eltype Float64 with indices static(1):static(6)×static(1):static(8):
 0.119847  0.649684  0.137111    0.649216  0.176489  0.946151    0.0680405  0.103388
 0.81982   0.332002  0.545564    0.729092  0.635139  0.882212    0.809322   0.018732
 0.730171  0.266168  0.219128    0.748357  0.820015  0.00645693  0.144222   0.591687
 0.881642  0.601935  0.871084    0.367133  0.621668  0.454102    0.662695   0.0535707
 0.572697  0.404497  0.533063    0.946217  0.850804  0.710305    0.0896228  0.98938
 0.981602  0.533421  0.00855031  0.873997  0.469507  0.28981     0.294725   0.866071

@ranocha
Copy link
Member Author

ranocha commented Mar 12, 2023

@chriselrod chriselrod enabled auto-merge (rebase) March 14, 2023 00:08
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (3a5f3d9) 56.32% compared to head (37ce3e7) 56.32%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #68   +/-   ##
=======================================
  Coverage   56.32%   56.32%           
=======================================
  Files           5        5           
  Lines         245      245           
=======================================
  Hits          138      138           
  Misses        107      107           
Impacted Files Coverage Δ
src/StrideArrays.jl 100.00% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

auto-merge was automatically disabled March 14, 2023 21:55

Rebase failed

@chriselrod chriselrod enabled auto-merge (rebase) May 9, 2023 23:18
auto-merge was automatically disabled May 10, 2023 14:58

Rebase failed

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