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: updates for breaking changes #164

Merged
merged 9 commits into from
Jul 5, 2024
Merged

fix: updates for breaking changes #164

merged 9 commits into from
Jul 5, 2024

Conversation

avik-pal
Copy link
Contributor

@avik-pal avik-pal commented Jul 3, 2024

CompatHelper isn't working #163, manually bumping the version to check if things work

@avik-pal
Copy link
Contributor Author

avik-pal commented Jul 4, 2024

cc @ChrisRackauckas can you approve the workflows to see if it works?

@avik-pal
Copy link
Contributor Author

avik-pal commented Jul 4, 2024

Any idea how to fix this?

Layer Tests: Error During Test at /mnt/research/lux/SimpleChains.jl/test/layer_tests.jl:35
  Test threw exception
  Expression: #= /mnt/research/lux/SimpleChains.jl/test/layer_tests.jl:35 =# @inferred(SimpleChains.valgrad(sc, SVector{5}(x), p)) isa Tuple{Float64, SVector{126, Float32}}
  MethodError: no method matching unsafe_convert(::Type{Ptr{Float64}}, ::Base.RefValue{LinearAlgebra.Adjoint{Float64, SVector{5, Float64}}})
  
  Closest candidates are:
    unsafe_convert(::Type{T}, ::T) where T<:Ptr
     @ Base essentials.jl:547
    unsafe_convert(::Type{Ptr{T}}, ::Ptr{Tuple{Vararg{T, N}}}) where {N, T}
     @ Base refpointer.jl:179
    unsafe_convert(::Union{Type{Ptr{T}}, Type{Ptr{Nothing}}}, ::Base.RefArray{T, A} where A<:(AbstractArray{T})) where T
     @ Base refpointer.jl:120
    ...
  
  Stacktrace:
    [1] memory_reference
      @ ~/.julia/packages/LayoutPointers/nNKcM/src/stridedpointers.jl:57 [inlined]
    [2] memory_reference
      @ ~/.julia/packages/LayoutPointers/nNKcM/src/stridedpointers.jl:18 [inlined]
    [3] stridedpointer_preserve
      @ ~/.julia/packages/LayoutPointers/nNKcM/src/stridedpointers.jl:105 [inlined]
    [4] macro expansion
      @ ~/.julia/packages/LoopVectorization/tIJUA/src/condense_loopset.jl:1179 [inlined]
    [5] dense!
      @ /mnt/research/lux/SimpleChains.jl/src/dense.jl:582 [inlined]
    [6] dense_param_update!(::TurboDense{true, Static.StaticInt{5}, typeof(tanh)}, Ā::StrideArraysCore.PtrArray{Float32, 2, (1, 2), Tuple{Static.StaticInt{5}, Static.StaticInt{6}}, Tuple{Nothing, Nothing}, Tuple{Static.StaticInt{1}, Static.StaticInt{1}}}, C̄::StrideArraysCore.PtrArray{Float32, 1, (1,), Tuple{Static.StaticInt{5}}, Tuple{Nothing}, Tuple{Static.StaticInt{1}}}, B::SVector{5, Float64})
      @ SimpleChains /mnt/research/lux/SimpleChains.jl/src/dense.jl:1007
    [7] pullback_param!(pg::Ptr{Float32}, td::TurboDense{true, Static.StaticInt{5}, typeof(tanh)}, C̄::StrideArraysCore.PtrArray{Float32, 1, (1,), Tuple{Static.StaticInt{5}}, Tuple{Nothing}, Tuple{Static.StaticInt{1}}}, B::SVector{5, Float64}, ::Ptr{Float32}, pu::Ptr{UInt8})
      @ SimpleChains /mnt/research/lux/SimpleChains.jl/src/dense.jl:987
    [8] chain_valgrad_entry!(pga::Nothing, pgp::Ptr{Float32}, arg::SVector{5, Float64}, layers::Tuple{TurboDense{true, Static.StaticInt{5}, typeof(tanh)}, TurboDense{false, Static.StaticInt{5}, typeof(tanh)}, TurboDense{true, Static.StaticInt{5}, typeof(relu)}, Dropout{VectorizedRNG.Xoshiro{2}}, TurboDense{false, Static.StaticInt{5}, typeof(relu)}, TurboDense{true, Static.StaticInt{2}, typeof(identity)}, TurboDense{false, Static.StaticInt{2}, typeof(identity)}, SquaredLoss{Vector{Float64}}}, p1::Ptr{Float32}, pu::Ptr{UInt8})
      @ SimpleChains /mnt/research/lux/SimpleChains.jl/src/simple_chain.jl:700
    [9] unsafe_valgrad!(c::SimpleChain{Tuple{Static.StaticInt{5}}, Tuple{TurboDense{true, Static.StaticInt{5}, typeof(tanh)}, TurboDense{false, Static.StaticInt{5}, typeof(tanh)}, TurboDense{true, Static.StaticInt{5}, typeof(relu)}, Dropout{VectorizedRNG.Xoshiro{2}}, TurboDense{false, Static.StaticInt{5}, typeof(relu)}, TurboDense{true, Static.StaticInt{2}, typeof(identity)}, TurboDense{false, Static.StaticInt{2}, typeof(identity)}, SquaredLoss{Vector{Float64}}}}, pu::Ptr{UInt8}, g::StrideArraysCore.PtrArray{Float32, 1, (1,), Tuple{Static.StaticInt{126}}, Tuple{Nothing}, Tuple{Static.StaticInt{1}}}, params::StrideArraysCore.StaticStrideArray{Float32, 1, (1,), Tuple{Static.StaticInt{126}}, Tuple{Nothing}, Tuple{Static.StaticInt{1}}, 126}, arg::SVector{5, Float64})
      @ SimpleChains /mnt/research/lux/SimpleChains.jl/src/simple_chain.jl:583
   [10] valgrad_core_sarray(c::SimpleChain{Tuple{Static.StaticInt{5}}, Tuple{TurboDense{true, Static.StaticInt{5}, typeof(tanh)}, TurboDense{false, Static.StaticInt{5}, typeof(tanh)}, TurboDense{true, Static.StaticInt{5}, typeof(relu)}, Dropout{VectorizedRNG.Xoshiro{2}}, TurboDense{false, Static.StaticInt{5}, typeof(relu)}, TurboDense{true, Static.StaticInt{2}, typeof(identity)}, TurboDense{false, Static.StaticInt{2}, typeof(identity)}, SquaredLoss{Vector{Float64}}}}, pu::Ptr{UInt8}, arg::SVector{5, Float64}, params::StrideArraysCore.StaticStrideArray{Float32, 1, (1,), Tuple{Static.StaticInt{126}}, Tuple{Nothing}, Tuple{Static.StaticInt{1}}, 126}, ::Static.StaticInt{126})
      @ SimpleChains /mnt/research/lux/SimpleChains.jl/src/simple_chain.jl:838
   [11] with_stack_memory
      @ /mnt/research/lux/SimpleChains.jl/src/memory.jl:25 [inlined]
   [12] with_memory
      @ /mnt/research/lux/SimpleChains.jl/src/memory.jl:48 [inlined]
   [13] valgrad(sc::SimpleChain{Tuple{Static.StaticInt{5}}, Tuple{TurboDense{true, Static.StaticInt{5}, typeof(tanh)}, TurboDense{false, Static.StaticInt{5}, typeof(tanh)}, TurboDense{true, Static.StaticInt{5}, typeof(relu)}, Dropout{VectorizedRNG.Xoshiro{2}}, TurboDense{false, Static.StaticInt{5}, typeof(relu)}, TurboDense{true, Static.StaticInt{2}, typeof(identity)}, TurboDense{false, Static.StaticInt{2}, typeof(identity)}, SquaredLoss{Vector{Float64}}}}, arg::SVector{5, Float64}, params::StrideArraysCore.StaticStrideArray{Float32, 1, (1,), Tuple{Static.StaticInt{126}}, Tuple{Nothing}, Tuple{Static.StaticInt{1}}, 126})
      @ SimpleChains /mnt/research/lux/SimpleChains.jl/src/simple_chain.jl:879
   [14] macro expansion
      @ ~/.julia/juliaup/julia-1.10.4+0.x64.linux.gnu/share/julia/stdlib/v1.10/Test/src/Test.jl:669 [inlined]
   [15] top-level scope
      @ /mnt/research/lux/SimpleChains.jl/test/layer_tests.jl:508

Static supports 1.10+ so I had to drop 1.6 here as well.

@avik-pal
Copy link
Contributor Author

avik-pal commented Jul 4, 2024

All tests should be passing on 1.10 now. Some @jet tests are failing due to base changes in 1.11

@avik-pal avik-pal changed the title Update compat entries fix: updates for breaking changes Jul 4, 2024
ChrisRackauckas
ChrisRackauckas previously approved these changes Jul 4, 2024
@ChrisRackauckas ChrisRackauckas enabled auto-merge July 4, 2024 04:57
@avik-pal
Copy link
Contributor Author

avik-pal commented Jul 4, 2024

what is finalize exactly checking?

@DilumAluthge
Copy link
Member

what is finalize exactly checking?

It just checks if any jobs failed.

@avik-pal
Copy link
Contributor Author

avik-pal commented Jul 4, 2024

nightly fails because JET doesn't work on nightly.

Copy link
Member

@andreasnoack andreasnoack left a comment

Choose a reason for hiding this comment

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

Thanks for looking into the failures here. A few comment. What are the upstream breaking changes that make the changes here necessary? Should they be fixed upstream? Do any of the fixes here require Static@1? If not then better to make the minimal fixes without making changes to compat bounds. Please change nightly to 1 for the tests with coverage and, if possible, keep testing on 1.6 without coverage.

src/utils.jl Outdated Show resolved Hide resolved
auto-merge was automatically disabled July 4, 2024 15:33

Head branch was pushed to by a user without write access

@avik-pal
Copy link
Contributor Author

avik-pal commented Jul 4, 2024

Should they be fixed upstream? Do any of the fixes here require Static@1

The removal of the + overloads from Static was intentional to reduce invalidations.

if possible, keep testing on 1.6 without coverage.

Static@1 needs 1.10, let me check if letting it install old versions is good enough

@avik-pal
Copy link
Contributor Author

avik-pal commented Jul 4, 2024

Static@1 needs 1.10, let me check if letting it install old versions is good enough

1.6 should pass now

Project.toml Outdated Show resolved Hide resolved
Copy link
Member

@andreasnoack andreasnoack left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this

@andreasnoack andreasnoack merged commit 5069dfa into PumasAI:main Jul 5, 2024
7 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.

4 participants