tomchor / Oceanostics.jl

Diagnostics for Oceananigans
https://tomchor.github.io/Oceanostics.jl/
MIT License
24 stars 8 forks source link

Create docs #124

Closed tomchor closed 1 year ago

tomchor commented 1 year ago

This PR creates a first draft of the docs.

Closes https://github.com/tomchor/Oceanostics.jl/issues/99 Closes https://github.com/tomchor/Oceanostics.jl/issues/120

Preview: https://tomchor.github.io/Oceanostics.jl/previews/PR124/

glwagner commented 1 year ago

Great!

glwagner commented 1 year ago

I think we'll be able to see the preview once the build succeeds

tomchor commented 1 year ago

I think we'll be able to see the preview once the build succeeds

Yup. It's building locally no problem, but I'm having a hard time making it build with GH actions. Not really sure where the error is coming from.

tomchor commented 1 year ago

Docs are building and we can preview them here: https://tomchor.github.io/Oceanostics.jl/previews/PR124/

tomchor commented 1 year ago

I think this is ready for review. Since this is a first step towards a docs page I opted for something simple: a home page with intro, installation, and a quick example; and three Oceananigans-style examples. I purposefully made the examples similar to Ocenanigans ones.

tomchor commented 1 year ago

For some reason there was again some problem with the manifest. The docs now pass and the preview can be foudn at: https://tomchor.github.io/Oceanostics.jl/previews/PR124/

navidcy commented 1 year ago

I see in netcdf metadata:

"Oceananigans"         => "This file was generated using "

Is a version missing there? Should we fix something in Oceananigans?

navidcy commented 1 year ago

The movie in the tilted bbl example is not showing up.

navidcy commented 1 year ago

The figures look a bit pixelated. I think svg works better! I'll find a code snippet in a bit and past here or push in the branch?

navidcy commented 1 year ago

Perhaps we may add a link to the corresponding Oceananigans example in the beginning?

tomchor commented 1 year ago

I see in netcdf metadata:

"Oceananigans"         => "This file was generated using "

Is a version missing there? Should we fix something in Oceananigans?

Hadn't noticed that. Going through some files I can see that most of them do have the version, while some don't. I think it's worth checking it out

tomchor commented 1 year ago

The movie in the tilted bbl example is not showing up.

Yeah I just noticed that. It shows up locally though. Any idea of what might be happening?

tomchor commented 1 year ago

The figures look a bit pixelated. I think svg works better! I'll find a code snippet in a bit and past here or push in the branch?

I don't see any pixelation here, so I guess it must be browser-dependent? In any case, feel free to either push some code or paste a snippet here to try and fix that. Whatever you prefer!

navidcy commented 1 year ago

This

CairoMakie.activate!(type = "svg")

will default CairoMakie outputting svg images which look much better. I don't think we have this in the examples as users can choose to output in whatever format they like. But perhaps in the make.jl file somewhere?

navidcy commented 1 year ago

The movie in the tilted bbl example is not showing up.

Yeah I just noticed that. It shows up locally though. Any idea of what might be happening?

I can have a closer look :)

tomchor commented 1 year ago

The movie in the tilted bbl example is not showing up.

Yeah I just noticed that. It shows up locally though. Any idea of what might be happening?

I can have a closer look :)

Ah, when re-running the example locally I got some errors which I'm guessing are causing this. I think I was testing this earlier with a REPL that some pre-definitions in it. Don't worry, I'll fix it. Thanks for catching that

navidcy commented 1 year ago

The GitHub action is building the docs using .develop(), i.e., with julia --project=docs/ -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate()'. This ignores any docs/Manifest.toml file.

What's the objective? Do we want to have a docs/Manifest.toml? Then the docs must be built like we do them in Oceananigans.jl.

navidcy commented 1 year ago

OK, I tried something. Since you removed the docs/Manifest.toml I removed what I also thought it was unnecessary. Let's see what happens. With d62918b and 2e40c91 now docs should be able to be built without the Manifest. Sorry if I messed things up. If docs fail I'll fix it.

navidcy commented 1 year ago

I noticed that .nc files were included in the previews dir in gh-branches. That would have made the repository size grow with time as they would be kept in the all docs for the tagged versions... I used Glob to delete output files before pushing the docs up.

tomchor commented 1 year ago

I noticed that .nc files were included in the previews dir in gh-branches. That would have made the repository size grow with time as they would be kept in the all docs for the tagged versions... I used Glob to delete output files before pushing the docs up.

Thanks!

I see the docs on CI where cancelled. Did you cancel them? Or were they cancelled automatically? In any case they were taking 6 hours... not a good sign.

I couldn't figure out how to make the docs compile locally after your changes to test this out. What command should I be using? It'd be good to include instructions for this in the docs/README.md file.

navidcy commented 1 year ago

I thought I canceled all but the latest built…

I use the same command as in the github action. I’ll add a remark!

navidcy commented 1 year ago

The docs take too long to built. How long do they need locally?

navidcy commented 1 year ago

I think contourf, e.g.,

https://github.com/tomchor/Oceanostics.jl/blob/37687aacea78d04642ccd0d083ed8f4873d7615c/docs/examples/tilted_bottom_boundary_layer.jl#L212-L214

is too slow with CairoMakie?

navidcy commented 1 year ago

I think

https://github.com/tomchor/Oceanostics.jl/blob/37687aacea78d04642ccd0d083ed8f4873d7615c/docs/examples/tilted_bottom_boundary_layer.jl#L212-L214

is too slow with CairoMakie?

I think these lines are the culprit! I changed to heat map and the docs built locally in 10min.... With contourf they got stuck in the last animation for 20min and I gave up.......

tomchor commented 1 year ago

I think https://github.com/tomchor/Oceanostics.jl/blob/37687aacea78d04642ccd0d083ed8f4873d7615c/docs/examples/tilted_bottom_boundary_layer.jl#L212-L214

is too slow with CairoMakie?

I think these lines are the culprit! I changed to heat map and the docs built locally in 10min.... With contourf they got stuck in the last animation for 20min and I gave up.......

I think you're right. Running the tilted BBL example script directly gives me this error:

ERROR: LoadError: AssertionError: length(positions) == length(colors)
Stacktrace:
  [1] draw_multi(primitive::Lines{Tuple{Vector{Point{2, Float32}}}}, ctx::Cairo.CairoContext, positions::Vector{Vec{2, Float32}}, colors::Vector{ColorTypes.RGBA{Float32}}, linewidths::Vector{Float32}, dash::Vector{Float64})
    @ CairoMakie ~/.julia/packages/CairoMakie/w6vFR/src/primitives.jl:165
  [2] draw_multi(primitive::Lines{Tuple{Vector{Point{2, Float32}}}}, ctx::Cairo.CairoContext, positions::Vector{Vec{2, Float32}}, colors::Vector{ColorTypes.RGBA{Float32}}, linewidth::Float32, dash::Vector{Float64})
    @ CairoMakie ~/.julia/packages/CairoMakie/w6vFR/src/primitives.jl:153
  [3] draw_atomic(scene::Scene, screen::CairoMakie.Screen{CairoMakie.IMAGE}, primitive::Union{Lines, LineSegments})
    @ CairoMakie ~/.julia/packages/CairoMakie/w6vFR/src/primitives.jl:63
  [4] draw_plot(scene::Scene, screen::CairoMakie.Screen{CairoMakie.IMAGE}, primitive::Lines{Tuple{Vector{Point{2, Float32}}}})
    @ CairoMakie ~/.julia/packages/CairoMakie/w6vFR/src/infrastructure.jl:108
  [5] draw_plot(scene::Scene, screen::CairoMakie.Screen{CairoMakie.IMAGE}, primitive::Combined{Makie.contour, Tuple{Vector{Float64}, Vector{Float64}, Matrix{Float32}}})
    @ CairoMakie ~/.julia/packages/CairoMakie/w6vFR/src/infrastructure.jl:113
  [6] cairo_draw(screen::CairoMakie.Screen{CairoMakie.IMAGE}, scene::Scene)
    @ CairoMakie ~/.julia/packages/CairoMakie/w6vFR/src/infrastructure.jl:49
  [7] colorbuffer(screen::CairoMakie.Screen{CairoMakie.IMAGE})
    @ CairoMakie ~/.julia/packages/CairoMakie/w6vFR/src/screen.jl:307
  [8] colorbuffer(screen::CairoMakie.Screen{CairoMakie.IMAGE}, format::Makie.ImageStorageFormat)
    @ Makie ~/.julia/packages/Makie/DekzU/src/display.jl:355
  [9] recordframe!(io::VideoStream)
    @ Makie ~/.julia/packages/Makie/DekzU/src/ffmpeg-util.jl:242
 [10] Record(func::var"#23#24", figlike::Figure, iter::UnitRange{Int64}; kw_args::Base.Pairs{Symbol, Any, Tuple{Symbol, Symbol}, NamedTuple{(:format, :framerate), Tuple{SubString{String}, Int64}}})
    @ Makie ~/.julia/packages/Makie/DekzU/src/recording.jl:168
 [11] record(func::Function, figlike::Figure, path::String, iter::UnitRange{Int64}; kw_args::Base.Pairs{Symbol, Int64, Tuple{Symbol}, NamedTuple{(:framerate,), Tuple{Int64}}})
    @ Makie ~/.julia/packages/Makie/DekzU/src/recording.jl:148
 [12] top-level scope
    @ ~/repos/Oceanostics.jl/docs/examples/tilted_bottom_boundary_layer.jl:227
 [13] include(fname::String)
    @ Base.MainInclude ./client.jl:476
 [14] top-level scope
    @ REPL[6]:1
 [15] top-level scope
    @ ~/.julia/packages/CUDA/BbliS/src/initialization.jl:52

And I guess Literate just hangs with the error instead of failing. This is weird though because this definitely used to work when I first drafted this example. I got videos with the contours and everything made by the same script. I also started seeing this error in an unrelated plot of mine this week, so I think this is a recently-introduced Makie bug.

I actually posted about it on the Makie slack channel but didn't get any useful info. If we can boil this down to a MWE and post it I think it would be helpful.

I'm removing the contours for now.

tomchor commented 1 year ago

Docs are building! @navidcy thanks so much for the help.

I think this is ready to merge. lmk if you think there's anything else that needs changing before that

navidcy commented 1 year ago

We forgot to put a link to the docs in the README. :)

tomchor commented 1 year ago

True! We can always polish it later. I'm also going to update the package description (in the top right) to include the docs (a la Oceananigans) after the docs finish building.