jlchan / StartUpDG.jl

Initializes and sets up reference elements and physical meshes for DG.
MIT License
28 stars 9 forks source link

Get rid of Colors as direct dependency #73

Closed ranocha closed 1 year ago

ranocha commented 1 year ago

Colors.jl loads FixedPointNumbers.jl which can easily invalidate quite a bit. Moreover, it adds quite a bunch of other dependencies that are only used for plotting. Thus, I moved the only recipe using something from Colors.jl to a weak dependency on Plots.jl using the framework described in https://pkgdocs.julialang.org/dev/creating-packages/#Conditional-loading-of-code-in-packages-(Extensions). To make it backward compatible with Julia < 1.9, I followed the advice there and used Requires.jl if necessary.

First tests suggest that this can reduce the time of using Trixi from

julia> @time @eval using Trixi
  8.004904 seconds (17.29 M allocations: 1.301 GiB, 5.61% gc time, 24.59% compilation time: 24% of which was recompilation)

with the latest release of StartUpDG.jl to

julia> @time @eval using Trixi
  7.637558 seconds (16.22 M allocations: 1.238 GiB, 5.32% gc time, 25.56% compilation time: 24% of which was recompilation)

using this branch, both on top of https://github.com/trixi-framework/Trixi.jl/pull/1322 and with the backports release 19.-beta3 branch linked there.

codecov[bot] commented 1 year ago

Codecov Report

Merging #73 (eb0a77a) into main (d753624) will decrease coverage by 0.04%. The diff coverage is 90.00%.

@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
- Coverage   96.68%   96.64%   -0.05%     
==========================================
  Files          20       21       +1     
  Lines        2295     2294       -1     
==========================================
- Hits         2219     2217       -2     
- Misses         76       77       +1     
Impacted Files Coverage Δ
src/mesh/simple_meshes.jl 97.62% <0.00%> (ø)
src/StartUpDG.jl 75.00% <66.66%> (-25.00%) :arrow_down:
src/TriangulatePlots.jl 96.00% <96.00%> (ø)
src/mesh/triangulate_utils.jl 93.87% <100.00%> (-0.72%) :arrow_down:
src/mesh/triangulate_example_meshes.jl 100.00% <0.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

ranocha commented 1 year ago

The CI error on Julia nightly (KeyError: key :plot_object not found) seems to be something internal to the recipes system?

ranocha commented 1 year ago

This seems like it needed a fairly significant change to work around the single function Colors.distinguishable_colors. Do you know of another way to distinguish different tagged boundaries on a domain? If so, we could use that and it might avoid some of this while still getting rid of Colors.jl.

No, I don't have a good idea either. This is the same approach I used in the package RootedTrees.jl that I maintain, see https://github.com/SciML/RootedTrees.jl/blob/ff066ce3d074533814c898e799db1f65cc94c532/src/RootedTrees.jl#L1406-L1409 and https://github.com/SciML/RootedTrees.jl/blob/ff066ce3d074533814c898e799db1f65cc94c532/src/plots.jl#L10. I you have a better idea, please let me know.

ranocha commented 1 year ago

I'd like to hold off on merging this until the nightly CI failure is resolved. I'm not sure why it appears only in Nightly...

I re-opened this PR to trigger CI again. Let's see whether it persists...

ranocha commented 1 year ago

I'd like to hold off on merging this until the nightly CI failure is resolved. I'm not sure why it appears only in Nightly...

It's not caused by this PR but also happens with the latest release of StartUpDG.jl and Julia 1.9.0-beta2:

julia> using Pkg; Pkg.activate(temp=true); Pkg.add(["StartUpDG", "Plots"])

julia> using StartUpDG, Plots

julia> rd = RefElemData(Tri(), N=3)
RefElemData for a degree 3 Polynomial() approximation on Tri() element.

julia> md = MeshData(uniform_mesh(Tri(),1)...,rd)
MeshData of dimension 2 with 2 elements

julia> meshIO = triangulate_domain(Scramjet())
TriangulateIO(
pointlist=[0.0 8.0 … 5.799478015774165 5.367285392933761; 0.0 0.0 … 0.9188005804169743 1.1423192862250717],
pointmarkerlist=Int32[1, 1, 3, 1, 1, 1, 1, 1, 1, 1  …  0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
trianglelist=Int32[9 69 … 210 850; 25 84 … 841 871; 264 85 … 871 841],
segmentlist=Int32[2 3 … 853 860; 224 240 … 834 214],
segmentmarkerlist=Int32[1, 3, 1, 2, 1, 1, 1, 1, 1, 1  …  1, 1, 1, 1, 1, 1, 1, 1, 1, 1],
holelist=[4.75; 0.55;;],
)

julia> plot(BoundaryTagPlotter(meshIO))
Error showing value of type Plots.Plot{Plots.GRBackend}:
ERROR: MethodError: no method matching firstindex(::Nothing)

Closest candidates are:
  firstindex(::Any, ::Any)
   @ Base abstractarray.jl:443
  firstindex(::Union{Tables.AbstractColumns, Tables.AbstractRow})
   @ Tables ~/.julia/packages/Tables/T7rHm/src/Tables.jl:182
  firstindex(::Union{ArrayInterfaceCore.BidiagonalIndex, ArrayInterfaceCore.TridiagonalIndex})
   @ ArrayInterfaceCore ~/.julia/packages/ArrayInterfaceCore/ByLA5/src/ArrayInterfaceCore.jl:664
  ...

Stacktrace:
  [1] _guess_best_legend_position(xl::Tuple{Float64, Float64}, yl::Tuple{Float64, Float64}, plt::Plots.Subplot{Plots.GRBackend}, weight::Int64)
    @ Plots ~/.julia/packages/Plots/nuwp4/src/utils.jl:1209
  [2] _guess_best_legend_position(xl::Tuple{Float64, Float64}, yl::Tuple{Float64, Float64}, plt::Plots.Subplot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/utils.jl:1197
  [3] _guess_best_legend_position(lp::Symbol, plt::Plots.Subplot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/utils.jl:1237
  [4] gr_legend_pos(sp::Plots.Subplot{Plots.GRBackend}, leg::NamedTuple{(:yoffset, :xoffset, :base_markersize, :base_factor, :has_title, :vertical, :entries, :space, :texth, :textw, :span, :pad, :dy, :dx, :w, :h), Tuple{Float64, Float64, Float64, Float64, Bool, Bool, Int64, Vararg{Float64, 9}}}, vp::Plots.GRViewport{Float64})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:1122
  [5] gr_add_legend(sp::Plots.Subplot{Plots.GRBackend}, leg::NamedTuple{(:yoffset, :xoffset, :base_markersize, :base_factor, :has_title, :vertical, :entries, :space, :texth, :textw, :span, :pad, :dy, :dx, :w, :h), Tuple{Float64, Float64, Float64, Float64, Bool, Bool, Int64, Vararg{Float64, 9}}}, viewport_area::Plots.GRViewport{Float64})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:993
  [6] gr_display(sp::Plots.Subplot{Plots.GRBackend}, w::Measures.AbsoluteLength, h::Measures.AbsoluteLength, vp_canvas::Plots.GRViewport{Float64})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:964
  [7] (::Plots.var"#528#529"{Int64, Int64, Plots.GRViewport{Float64}})(sp::Plots.Subplot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:680
  [8] foreach(f::Plots.var"#528#529"{Int64, Int64, Plots.GRViewport{Float64}}, itr::Vector{Plots.Subplot})
    @ Base ./abstractarray.jl:3067
  [9] gr_display(plt::Plots.Plot{Plots.GRBackend}, dpi_factor::Int64)
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:680
 [10] gr_display
    @ ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:650 [inlined]
 [11] #574
    @ ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:2044 [inlined]
 [12] withenv(::Plots.var"#574#576"{Plots.Plot{Plots.GRBackend}}, ::Pair{String, String}, ::Vararg{Pair{String}})
    @ Base ./env.jl:197
 [13] _display(plt::Plots.Plot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:2043
 [14] display(#unused#::Plots.PlotsDisplay, plt::Plots.Plot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/output.jl:174
 [15] display(x::Any)
    @ Base.Multimedia ./multimedia.jl:340
 [16] #invokelatest#2
    @ ./essentials.jl:816 [inlined]
 [17] invokelatest
    @ ./essentials.jl:813 [inlined]
 [18] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
    @ REPL ~/Software/julia-1.9.0-beta2/share/julia/stdlib/v1.9/REPL/src/REPL.jl:302
 [19] (::REPL.var"#57#58"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
    @ REPL ~/Software/julia-1.9.0-beta2/share/julia/stdlib/v1.9/REPL/src/REPL.jl:284
 [20] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL ~/Software/julia-1.9.0-beta2/share/julia/stdlib/v1.9/REPL/src/REPL.jl:551
 [21] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
    @ REPL ~/Software/julia-1.9.0-beta2/share/julia/stdlib/v1.9/REPL/src/REPL.jl:282
 [22] (::REPL.var"#do_respond#80"{Bool, Bool, REPL.var"#93#103"{REPL.LineEditREPL, REPL.REPLHistoryProvider}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
    @ REPL ~/Software/julia-1.9.0-beta2/share/julia/stdlib/v1.9/REPL/src/REPL.jl:893
 [23] #invokelatest#2
    @ ./essentials.jl:816 [inlined]
 [24] invokelatest
    @ ./essentials.jl:813 [inlined]
 [25] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/Software/julia-1.9.0-beta2/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:2644
 [26] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL ~/Software/julia-1.9.0-beta2/share/julia/stdlib/v1.9/REPL/src/REPL.jl:1293
 [27] (::REPL.var"#62#68"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ./task.jl:514
ranocha commented 1 year ago

How shall we proceed? I see two options

jlchan commented 1 year ago

To clarify - the CI for the current version passes on Julia nightly, but the actual plotting fails on Julia nightly?

In terms of proceeding, I'm OK with either option. If you don't mind filing an issue upstream, we can merge this anytime.

ranocha commented 1 year ago

To clarify - the CI for the current version passes on Julia nightly, but the actual plotting fails on Julia nightly?

Yes, exactly

In terms of proceeding, I'm OK with either option. If you don't mind filing an issue upstream, we can merge this anytime.

I will file an issue :+1: I think it would be nice to go ahead to test it in the wild and get reduced latency.

jlchan commented 1 year ago

Sounds good. I will go ahead and merge this then.

jlchan commented 1 year ago

Are you OK just testing this out from the main branch? Releasing a new version would get blocked due to the nightly CI failure

ranocha commented 1 year ago

Ok :+1:

ranocha commented 1 year ago

This error already happens on Julia 1.8.3?

julia> using Pkg; Pkg.activate(temp=true); Pkg.add(["StartUpDG", "Plots"])
  Activating new project at `/tmp/jl_yrB65R`
    Updating registry at `~/.julia/registries/General.toml`
   Resolving package versions...
   Installed HTTP ─ v1.7.2
    Updating `/tmp/jl_yrB65R/Project.toml`
  [91a5bcdd] + Plots v1.38.2
  [472ebc20] + StartUpDG v0.15.2
...
julia> using StartUpDG, Plots

julia> begin
           rd = RefElemData(Tri(), N = 3)
           md = MeshData(uniform_mesh(Tri(), 1)..., rd)
           meshIO = triangulate_domain(Scramjet())
           plot(BoundaryTagPlotter(meshIO))
       end
Error showing value of type Plots.Plot{Plots.GRBackend}:
ERROR: MethodError: no method matching firstindex(::Nothing)
Closest candidates are:
  firstindex(::Any, ::Any) at abstractarray.jl:402
  firstindex(::Union{Tables.AbstractColumns, Tables.AbstractRow}) at ~/.julia/packages/Tables/T7rHm/src/Tables.jl:182
  firstindex(::Union{ArrayInterfaceCore.BidiagonalIndex, ArrayInterfaceCore.TridiagonalIndex}) at ~/.julia/packages/ArrayInterfaceCore/ByLA5/src/ArrayInterfaceCore.jl:664
  ...
Stacktrace:
  [1] _guess_best_legend_position(xl::Tuple{Float64, Float64}, yl::Tuple{Float64, Float64}, plt::Plots.Subplot{Plots.GRBackend}, weight::Int64)
    @ Plots ~/.julia/packages/Plots/nuwp4/src/utils.jl:1209
  [2] _guess_best_legend_position(xl::Tuple{Float64, Float64}, yl::Tuple{Float64, Float64}, plt::Plots.Subplot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/utils.jl:1197
  [3] _guess_best_legend_position(lp::Symbol, plt::Plots.Subplot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/utils.jl:1237
  [4] gr_legend_pos(sp::Plots.Subplot{Plots.GRBackend}, leg::NamedTuple{(:yoffset, :xoffset, :base_markersize, :base_factor, :has_title, :vertical, :entries, :space, :texth, :textw, :span, :pad, :dy, :dx, :w, :h), Tuple{Float64, Float64, Float64, Float64, Bool, Bool, Int64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64}}, vp::Plots.GRViewport{Float64})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:1122
  [5] gr_add_legend(sp::Plots.Subplot{Plots.GRBackend}, leg::NamedTuple{(:yoffset, :xoffset, :base_markersize, :base_factor, :has_title, :vertical, :entries, :space, :texth, :textw, :span, :pad, :dy, :dx, :w, :h), Tuple{Float64, Float64, Float64, Float64, Bool, Bool, Int64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64, Float64}}, viewport_area::Plots.GRViewport{Float64})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:993
  [6] gr_display(sp::Plots.Subplot{Plots.GRBackend}, w::Measures.AbsoluteLength, h::Measures.AbsoluteLength, vp_canvas::Plots.GRViewport{Float64})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:964
  [7] (::Plots.var"#526#527"{Int64, Int64, Plots.GRViewport{Float64}})(sp::Plots.Subplot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:680
  [8] foreach(f::Plots.var"#526#527"{Int64, Int64, Plots.GRViewport{Float64}}, itr::Vector{Plots.Subplot})
    @ Base ./abstractarray.jl:2774
  [9] gr_display(plt::Plots.Plot{Plots.GRBackend}, dpi_factor::Int64)
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:680
 [10] gr_display
    @ ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:650 [inlined]
 [11] #572
    @ ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:2044 [inlined]
 [12] withenv(::Plots.var"#572#574"{Plots.Plot{Plots.GRBackend}}, ::Pair{String, String}, ::Vararg{Pair{String}})
    @ Base ./env.jl:172
 [13] _display(plt::Plots.Plot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/backends/gr.jl:2043
 [14] display(#unused#::Plots.PlotsDisplay, plt::Plots.Plot{Plots.GRBackend})
    @ Plots ~/.julia/packages/Plots/nuwp4/src/output.jl:174
 [15] display(x::Any)
    @ Base.Multimedia ./multimedia.jl:328
 [16] #invokelatest#2
    @ ./essentials.jl:729 [inlined]
 [17] invokelatest
    @ ./essentials.jl:726 [inlined]
 [18] print_response(errio::IO, response::Any, show_value::Bool, have_color::Bool, specialdisplay::Union{Nothing, AbstractDisplay})
    @ REPL ~/Software/julia-1.8.3/share/julia/stdlib/v1.8/REPL/src/REPL.jl:296
 [19] (::REPL.var"#45#46"{REPL.LineEditREPL, Pair{Any, Bool}, Bool, Bool})(io::Any)
    @ REPL ~/Software/julia-1.8.3/share/julia/stdlib/v1.8/REPL/src/REPL.jl:278
 [20] with_repl_linfo(f::Any, repl::REPL.LineEditREPL)
    @ REPL ~/Software/julia-1.8.3/share/julia/stdlib/v1.8/REPL/src/REPL.jl:521
 [21] print_response(repl::REPL.AbstractREPL, response::Any, show_value::Bool, have_color::Bool)
    @ REPL ~/Software/julia-1.8.3/share/julia/stdlib/v1.8/REPL/src/REPL.jl:276
 [22] (::REPL.var"#do_respond#66"{Bool, Bool, REPL.var"#77#87"{REPL.LineEditREPL, REPL.REPLHistoryProvider}, REPL.LineEditREPL, REPL.LineEdit.Prompt})(s::REPL.LineEdit.MIState, buf::Any, ok::Bool)
    @ REPL ~/Software/julia-1.8.3/share/julia/stdlib/v1.8/REPL/src/REPL.jl:857
 [23] #invokelatest#2
    @ ./essentials.jl:729 [inlined]
 [24] invokelatest
    @ ./essentials.jl:726 [inlined]
 [25] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/Software/julia-1.8.3/share/julia/stdlib/v1.8/REPL/src/LineEdit.jl:2510
 [26] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL ~/Software/julia-1.8.3/share/julia/stdlib/v1.8/REPL/src/REPL.jl:1248
 [27] (::REPL.var"#49#54"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ./task.jl:484
jlchan commented 1 year ago

That is odd. Can you file an issue and I'll take a look this weekend?