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

Incompatible with Julia 1.7? #232

Closed
maximilian-gelbrecht opened this issue Jan 19, 2023 · 2 comments · Fixed by #246
Closed

Incompatible with Julia 1.7? #232

maximilian-gelbrecht opened this issue Jan 19, 2023 · 2 comments · Fixed by #246
Labels
bug 🐞 Something isn't working

Comments

@maximilian-gelbrecht
Copy link
Member

I tested the package with two different Julia 1.7 installations and both times got the same precompilation error.
With Julia 1.8.x it does work though.

┌ Info: Precompiling SpeedyWeather [9e226e20-d153-4fed-8a5b-493def4f21a9]
└ @ Base loading.jl:1423
ERROR: LoadError: MethodError: no method matching var"@inline"(::LineNumberNode, ::Module)
Closest candidates are:
  var"@inline"(::LineNumberNode, ::Module, ::Any) at /p/system/packages/julia/1.7.0/share/julia/base/expr.jl:201
Stacktrace:
  [1] include(mod::Module, _path::String)
    @ Base ./Base.jl:418
  [2] include(x::String)
    @ SpeedyWeather ~/.julia/packages/SpeedyWeather/Rl2dF/src/SpeedyWeather.jl:1
  [3] top-level scope
    @ ~/.julia/packages/SpeedyWeather/Rl2dF/src/Grids/Grids.jl:3
  [4] include(mod::Module, _path::String)
    @ Base ./Base.jl:418
  [5] include(x::String)
    @ SpeedyWeather ~/.julia/packages/SpeedyWeather/Rl2dF/src/SpeedyWeather.jl:1
  [6] top-level scope
    @ ~/.julia/packages/SpeedyWeather/Rl2dF/src/SpeedyWeather.jl:89
  [7] include
    @ ./Base.jl:418 [inlined]
  [8] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt64}}, source::String)
    @ Base ./loading.jl:1318
  [9] top-level scope
    @ none:1
 [10] eval
    @ ./boot.jl:373 [inlined]
 [11] eval(x::Expr)
    @ Base.MainInclude ./client.jl:453
 [12] top-level scope
    @ none:1
in expression starting at /home/maxgelbr/.julia/packages/SpeedyWeather/Rl2dF/src/Grids/grids_general.jl:148
in expression starting at /home/maxgelbr/.julia/packages/SpeedyWeather/Rl2dF/src/Grids/grids_general.jl:147
in expression starting at /home/maxgelbr/.julia/packages/SpeedyWeather/Rl2dF/src/Grids/Grids.jl:3
in expression starting at /home/maxgelbr/.julia/packages/SpeedyWeather/Rl2dF/src/SpeedyWeather.jl:1
@maximilian-gelbrecht maximilian-gelbrecht added the bug 🐞 Something isn't working label Jan 19, 2023
@milankl
Copy link
Member

milankl commented Jan 19, 2023

I've added this method last week

function eachgridpoint(grid1::Grid,grids::Grid...) where {Grid<:AbstractGrid}
@inline
n = length(grid1)
Base._all_match_first(X->length(X),n,grid1,grids...) || throw(BoundsError)
return eachgridpoint(grid1)
end

which I adapted from eachindex in base/abstractarray.jl

https://github.com/JuliaLang/julia/blob/e7c339fdec26148be24370de31ff91a7f40b001b/base/abstractarray.jl#L383-L389

I didn't know it was Julia ^1.8 but it helped to reduce allocations throughout the entire model that were created before. maybe @inline should be placed elsewhere for compatibility?

@milankl
Copy link
Member

milankl commented Jan 19, 2023

Apparently an @inline "callsite annotation" requires Julia 1.8, maybe we can just write

@inline function eachgridpoint(...)
    # body
end

instead?

@milankl milankl linked a pull request Feb 7, 2023 that will close this issue
@milankl milankl closed this as completed Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants