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

Documentation Overhaul #231

Merged
merged 43 commits into from
Jun 6, 2024
Merged

Documentation Overhaul #231

merged 43 commits into from
Jun 6, 2024

Conversation

GeorgeR227
Copy link
Collaborator

All documentation pages are being reviewed and improved to clean up our pages and to cut down on the documentation building time.

@GeorgeR227 GeorgeR227 added the documentation Improvements or additions to documentation label May 24, 2024
@GeorgeR227 GeorgeR227 linked an issue May 24, 2024 that may be closed by this pull request
Includes overview, bc_debug and equations.
@GeorgeR227
Copy link
Collaborator Author

Benchmarks for original docs build:

navier_stokes/ns - 0 seconds
halmo - 63 seconds
overview - 28 seconds (locally run)
klausmeier - 233 seconds
ice_dynamics - 68 seconds
grigoriev - 74 seconds
budyko_sellers_halfar - 57 seconds
cism - 32 seconds
nhs - 404 seconds
equations - 1 second
bc_debug - 95 seconds
poiseuille - 29 seconds

We should be avoiding hosting images on other web servers. I've moved most to be locally stored.
The sharp used in the 1D sim needs to be cleaned up.
Some doc pages still remain but these have special circumstances which need to be investigated. There are also some overall fixes still to be made in some docs but these have been pointed out.
@GeorgeR227 GeorgeR227 marked this pull request as ready for review May 29, 2024 18:53
Copy link
Member

@jpfairbanks jpfairbanks left a comment

Choose a reason for hiding this comment

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

Great work @GeorgeR227, this is almost done. I added some inline comments.

docs/src/budyko_sellers_halfar.md Outdated Show resolved Hide resolved
docs/src/budyko_sellers_halfar.md Show resolved Hide resolved
docs/make.jl Outdated Show resolved Hide resolved
docs/src/cism/cism.md Outdated Show resolved Hide resolved
docs/make.jl Outdated Show resolved Hide resolved
docs/src/ice_dynamics.md Outdated Show resolved Hide resolved
docs/src/ice_dynamics.md Show resolved Hide resolved
docs/src/ice_dynamics.md Show resolved Hide resolved
docs/src/klausmeier/klausmeier.md Show resolved Hide resolved
docs/src/nhs/nhs.md Outdated Show resolved Hide resolved
docs/src/ice_dynamics.md Outdated Show resolved Hide resolved
@lukem12345
Copy link
Member

lukem12345 commented May 31, 2024 via email

The one that runs now is a lightweight version of the full page. The full script, along with the old page, is still included but are not used.
@GeorgeR227
Copy link
Collaborator Author

Benchmark times as of this last commit. These are built locally, not on HiPerGator like in #231 (comment).

navier_stokes/ns - 0 seconds
halmo - removed
overview - 30 seconds
klausmeier - 24 seconds
ice_dynamics - 42 seconds
grigoriev - 21 seconds
budyko_sellers_halfar - 25 seconds
cism - 27 seconds
nhs - 11 seconds (lite version)
equations - 1 second
bc_debug - 22 seconds
poiseuille - 9 seconds
cahn-hilliard - 165 seconds

docs/docinfo.jl Outdated Show resolved Hide resolved
docs/make.jl Outdated Show resolved Hide resolved
docs/src/bc_debug.md Show resolved Hide resolved
docs/src/bc_debug.md Outdated Show resolved Hide resolved
docs/src/budyko_sellers_halfar.md Show resolved Hide resolved
docs/src/grigoriev/grigoriev.md Outdated Show resolved Hide resolved
docs/src/grigoriev/grigoriev.md Outdated Show resolved Hide resolved
docs/src/grigoriev/grigoriev.md Outdated Show resolved Hide resolved
docs/src/ice_dynamics.md Show resolved Hide resolved
docs/src/ice_dynamics.md Show resolved Hide resolved
@GeorgeR227
Copy link
Collaborator Author

For some reason halmo.md is breaking the docs build. When I remove it, the docs build fine and fairly quickly. The moment I re-include it the docs just seem to perpetually hang. I've tried building that page by itself and it seems to build fine so I'm not sure what the issue is.

@GeorgeR227
Copy link
Collaborator Author

I'm going to try going through all the files and see which ones could be causing issues with halmo.md. If I can't diagnose it then I'd want to just remove it for now from the pages so we can merge this PR.

@lukem12345
Copy link
Member

Theory: There is a race condition.

The issue occurs (sometimes) when we generate a .jl file. Perhaps Documenter is seeing the .jl file and doing its Documenter black magic on it before the file is finished writing etc. etc.

@GeorgeR227
Copy link
Collaborator Author

Well the .jl file is removed and it's still dead-locking. Leading theory for me right now is that it is something to do with the save_vorticity function.

@GeorgeR227
Copy link
Collaborator Author

I'm not sure what to say but it seems like save_vorticity may have been the cause. I've run the docs multiple times now and it seems to work.

@GeorgeR227
Copy link
Collaborator Author

Pretty sure the issue occurs when we actually create the gif. Still not sure as to why.
image

@lukem12345
Copy link
Member

lukem12345 commented Jun 4, 2024 via email

We should use `xlabel` from here on instead of `Bottom()`
@GeorgeR227
Copy link
Collaborator Author

@lukem12345 You were right about ffmpeg.exe this seems to be deadlocked and is the only program with this handle.
image

This'll need to be revisited since this is just a workaround.
@GeorgeR227
Copy link
Collaborator Author

I've opted to just remove the offending code in halmo.md because I fear the hanging issue is not something we really have the ability to fix. I also still don't know the root cause of the hanging.

@GeorgeR227 GeorgeR227 merged commit 3e90b45 into main Jun 6, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

General Documentation Fixes
3 participants