Fixed doc build error by specifying DISPLAY, other workflow improvements

[jonathanfischer97]

Fix Documentation Build Failure Due to Missing DISPLAY Setting for GLMakie

Main fixes

1. Explicit DISPLAY Setting: Updated the xvfb-run command to explicitly set DISPLAY=:0 directly in the command instead of using an environment variable. This ensures that the virtual display is consistently set up, avoiding the initialization errors with GLMakie.

2. Removed --auto-servernum: Since DISPLAY is explicitly set to :0, the --auto-servernum argument is no longer necessary.

Other improvements

These are other minor improvements I added just from looking at Makie.jl's workflows, so not strictly necessary and I can remove them if you'd like:

1. Caching: added caching to improve build speed, specifically:

   • ~/.julia/artifacts: Stores binary dependencies, so faster builds without redundant downloads.
   • ~/.julia/compiled: Contains precompiled packages, speeding up package loading.

2. Artifact Upload Step: Added an artifact upload step to store the generated documentation (./docs/__site) as a downloadable artifact. This allows for easy inspection of the generated documentation after each build, even before deployment. Makie.jl does this, thought it might be useful here too for adding/debugging docs in the future.

[isaacsas]

What is this part about the keys doing? The default SciML version seems to be just

      - uses: julia-actions/cache@v1
        if: "${{ inputs.cache }}"
        with:
          token: "${{ secrets.GITHUB_TOKEN }}"

see https://github.com/SciML/.github/blob/master/.github/workflows/documentation.yml#L62

[jonathanfischer97]

The key and restore-keys are used to manage caching in the workflow. The key uniquely identifies the cached data, which in our case artifacts and compiled code, to the project and manifest files. The restore-keys allow the workflow to use partially matching caches if the exact key is not found, providing some speedup even if the environment isn't an exact match, like if there were minor version changes or something.

But the SciML version is simpler and these specific keys might not be necessary, as I think the default key made my julia-actions/cache@v2 suffices https://github.com/julia-actions/cache?tab=readme-ov-file#cache-keys , and I'm sure the people that wrote the SciML version are more informed than me on this. I'll push a simpler version.

[jonathanfischer97]

Pushed some changes in response to your review, but I can just split off these additions into a separate PR and just leave the main GLMakie precompilation fix, if you'd prefer.

[isaacsas]

There seem to be issues in the doc build:

https://github.com/SciML/Catalyst.jl/actions/runs/11489840125/job/31979393974?pr=1086#step:6:1975

https://github.com/SciML/Catalyst.jl/actions/runs/11489840125/job/31979393974?pr=1086#step:7:400

https://github.com/SciML/Catalyst.jl/actions/runs/11489840125/job/31979393974?pr=1086#step:8:10

It is also strange none of these caused the build to fail?

[jonathanfischer97]

Strange indeed. Looking into this, apologies that this PR has been so difficult

[isaacsas]

@jonathanfischer97 when I download the artifact there are no images in the updated tutorial beyond your animation. Is this expected in the artifact?

Also, I really like having a way to download and preview the PR via the artifact, thanks for adding! Is there a way to have it deleted upon merging though to keep space usage down? (Alternatively, do these artifacts get written over between versions or are we going to store artifacts for every PR run going forward?)

[isaacsas]

Also, nothing to apologize for -- I have no understanding of Github actions at all, and I think you've added some nice functionality with this! The improved doc build times due to the caching is awesome. I wonder if we can have something similar for the CI runs too.

[jonathanfischer97]

@jonathanfischer97 when I download the artifact there are no images in the updated tutorial beyond your animation. Is this expected in the artifact?

Also, I really like having a way to download and preview the PR via the artifact, thanks for adding! Is there a way to have it deleted upon merging though to keep space usage down? (Alternatively, do these artifacts get written over between versions or are we going to store artifacts for every PR run going forward?)

According to the docs, the default behavior is that the artifact is retained for 90 days at a medium level of compression, and does not overwrite (if artifact of same name exists the action fails).

• But we can set a lower retention period (5 days?)
• We can also increase the compression (default is 6, with 1 lowest and 9 highest)
• And of course we can set overwrite = true. I'm not quite sure why this isn't the default behavior, I guess in order to preserve a record of each build.

[isaacsas]

If it is easy to do perhaps drop to a week retention? If not, 90 days is ok. I just want to know there is an upper bound. Thanks!

[jonathanfischer97]

Also, nothing to apologize for -- I have no understanding of Github actions at all, and I think you've added some nice functionality with this! The improved doc build times due to the caching is awesome. I wonder if we can have something similar for the CI runs too.

I'm very glad to help. There are several interesting actions in the julia-actions organization that might be useful for this package https://github.com/julia-actions, I'd be happy to continue improving our workflows after this latest PR merges.

[isaacsas]

That'd be great. I switched CI to the SciML default with a few tweaks, but I don't think caching is doing much there so perhaps it can be tweaked too.

[isaacsas]

BTW, tests are going to fail, but as long as the docs build without issues we can merge this when you are done updating (and the artifact is working properly). (The test failures are all related to some recent remake updates in SciMLBase I think, so should be unrelated to this PR.)

[jonathanfischer97]

BTW, tests are going to fail, but as long as the docs build without issues we can merge this when you are done updating (and the artifact is working properly). (The test failures are all related to some recent remake updates in SciMLBase I think, so should be unrelated to this PR.)

Oh gosh, not more remake changes, the changes to that one method have wreaked havoc on my codebase since MTK 14.0 haha.

Understood, if the plots are rendered and embedded properly with the current job, I'm going to push one more change that implements higher compression and lower retention time for the artifact, and if everything works then we can merge.

[isaacsas]

I’m sorry about remake causing you issues. We are discussing starting to upper bound certain dependencies that cause more frequent problems when we release v15. The idea being that we would ensure everything is working on a given version of them at release time, and then by upper bounding them they won’t get updated on you and break your code.

[isaacsas]

FWIW, I’ve also found things have become too unstable the past year (and I think @TorkelE would agree). If you have further suggestions for improving stability please do let us know. We have been brainstorming what we can do on our end about it.

[jonathanfischer97]

Looks like it worked! Apparently this error in the build step does not obstruct the plots being embedded properly:

└ @ Documenter.HTMLWriter ~/.julia/packages/Documenter/C1XEF/src/html/HTMLWriter.jl:807
[ Info: Automatic `version="14.4.1"` for inventory from ../Project.toml
┌ Info: Deployment criteria for deploying preview build from GitHub Actions:
│ - ✔ ENV["GITHUB_REPOSITORY"]="SciML/Catalyst.jl" occurs in repo="github.com/SciML/Catalyst.jl.git"
│ - ✔ ENV["GITHUB_REF"] corresponds to a PR number
│ - ✘ PR originates from the same repository
│ - ✔ `push_preview` keyword argument to deploydocs is `true`
│ - ✔ ENV["GITHUB_ACTOR"] exists and is non-empty
│ - ✔ ENV["GITHUB_TOKEN"] exists and is non-empty
└ Deploying: ✘
┌ Warning: error closing screen
│   exception =
│    GLFWError (NOT_INITIALIZED): The GLFW library is not initialized
│    Stacktrace:
│     [1] _ErrorCallbackWrapper(code::Int32, description::Cstring)
│       @ GLFW ~/.julia/packages/GLFW/wmoTL/src/callback.jl:43
│     [2] HideWindow
│       @ ~/.julia/packages/GLFW/wmoTL/src/glfw3.jl:686 [inlined]
│     [3] set_visibility!
│       @ ~/.julia/packages/GLFW/wmoTL/src/glfw3.jl:436 [inlined]
│     [4] set_screen_visibility!
│       @ ~/.julia/packages/GLMakie/o5Nad/src/screen.jl:436 [inlined]
│     [5] set_screen_visibility!
│       @ ~/.julia/packages/GLMakie/o5Nad/src/screen.jl:431 [inlined]
│     [6] close(screen::GLMakie.Screen{GLFW.Window}; reuse::Bool)
│       @ GLMakie ~/.julia/packages/GLMakie/o5Nad/src/screen.jl:663
│     [7] close
│       @ ~/.julia/packages/GLMakie/o5Nad/src/screen.jl:661 [inlined]
│     [8] renderloop(screen::GLMakie.Screen{GLFW.Window})
│       @ GLMakie ~/.julia/packages/GLMakie/o5Nad/src/screen.jl:982
│     [9] (::GLMakie.var"#71#72"{GLMakie.Screen{GLFW.Window}})()
│       @ GLMakie ~/.julia/packages/GLMakie/o5Nad/src/screen.jl:827
└ @ GLMakie ~/.julia/packages/GLMakie/o5Nad/src/screen.jl:984

Go ahead and merge, if you please. Though in the future, we might consider adding a clean-up workflow for the docs to remove stale documentation, as done here: https://github.com/JuliaGeodynamics/GeophysicalModelGenerator.jl/blob/main/.github/workflows/DocPreviewCleanup.yml and discussed here: https://documenter.juliadocs.org/stable/man/hosting/#Cleaning-up-gh-pages

[jonathanfischer97]

FWIW, I’ve also found things have become too unstable the past year (and I think @TorkelE would agree). If you have further suggestions for improving stability please do let us know. We have been brainstorming what we can do on our end about it.

Quite understandable, and the reasons behind the changes make sense of course. But I certainly have my own user feedback, which I can forward to you both over slack if it's of any instruction.

[isaacsas]

Thanks! And yes, a cleanup workflow would be nice.