Skip to content

Conversation

kmdeck
Copy link
Member

@kmdeck kmdeck commented Aug 5, 2025

Purpose

Improve documentation for fluxnet and global runs; simplify table of contents in docs
preview: https://clima.github.io/ClimaLand.jl/previews/PR1288/

Bucket Plots:
bucket_figures.pdf
bucket_annual_timeseries.pdf

Snow land fluxnet:
US_NR1_variable_timeseries.pdf
US_NR1_diurnal_timeseries.pdf

Soil canopy
US_MOz_diurnal_timeseries.pdf

To-do

Future PRs
- Change default plotting to png - otherwise we cannot show pdf in doc markdown

Content

docs:

  • Rearrange docs a bit: create a fluxnet simulation group, and a global sim group, in addition to standalone model group
  • add fluxnet documentation (soil canopy and snowy land runs, visualization, forcing data)
  • add global run documentation (bucket + snowy land)
  • added stand in pages for things we know we need to add
    To review: @juliasloan25 all docs/ changes and src/integrated/soil_canopy_model.jl
    @AlexisRenchon all docs/ changes and src/integrated/soil_canopy_model.jl

fluxnet and simulations:

  • Add make_set_fluxnet_ic function to hide closure from user and be more in line with how we create the ic function in the global case
  • added start_date to Simulation struct (used in plotting fluxnet runs)
  • small other changes
  • @imreddyTeja could review: - ozark_pft.jl
  • ozark_soilsnow.jl
  • run_fluxnet.jl
  • ozark_conservation.jl
  • ext/FluxnetSimulationsExt.jl
  • ext/LandSimulationVisualizationExt.jl
  • ext/fluxnet_simulations/initial_conditions.jl
  • ext/land_sim_vis/plotting_utils.jl
  • src/simulations/Simulations.jl

Moved to PR 1315:

  • src/standalone/Vegetation/Canopy.jl
  • test/integrated/full_land.jl
    -test/integrated/full_land_utils.jl
  • test/standalone/Soil/Biogeochemistry/biogeochemistry_module.jl

  • I have read and checked the items on the review checklist.

@kmdeck kmdeck force-pushed the kd/fluxnet_documentation branch from e62762d to 3a03605 Compare August 5, 2025 18:32
@juliasloan25
Copy link
Member

We should support periodic BC for fluxnet forcing - can we do that?

I don't completely understand how we read in the fluxnet forcing. We can have periodic BCs if we use the TVI constructor we use for prescribed_forcing_era5 instead of the one we use for prescribed_forcing_fluxnet, but I'm not sure if there's something prohibiting us from using that.

@kmdeck kmdeck force-pushed the kd/fluxnet_documentation branch from 1b8f227 to 18f63f5 Compare August 10, 2025 18:46
@kmdeck
Copy link
Member Author

kmdeck commented Aug 10, 2025

We should support periodic BC for fluxnet forcing - can we do that?

I don't completely understand how we read in the fluxnet forcing. We can have periodic BCs if we use the TVI constructor we use for prescribed_forcing_era5 instead of the one we use for prescribed_forcing_fluxnet, but I'm not sure if there's something prohibiting us from using that.

I wasnt sure because when I worked on this the time for the TimeVaryingInput0D had to be a float, in which case a periodic calendar doesnt make sense...but perhaps we can do this now. Ill try in a later PR

@kmdeck kmdeck changed the title Fluxnet documentation Fluxnet and global run documentation Aug 10, 2025
@kmdeck kmdeck mentioned this pull request Aug 4, 2025
17 tasks
@kmdeck kmdeck force-pushed the kd/fluxnet_documentation branch 3 times, most recently from 6f7648f to a255721 Compare August 13, 2025 01:35
@kmdeck kmdeck marked this pull request as ready for review August 13, 2025 02:10
Copy link
Member

@AlexisRenchon AlexisRenchon left a comment

Choose a reason for hiding this comment

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

This is a dramatic improvement of ClimaLand documentation, thank you Kat!
I left a bunch of comments below, I don't think we need to address them all in this PR, I leave it up to you.
I can go through another review later on today if needed, let me know when you think it is ready to merge!

Note: the getting_started.md page (not modified in this PR) has a typo, we could correct it here. Line 17, "journet" instead of "journey".

In getting_started.md, there's an error due to "About.jl" not being installed. I know we removed it a while ago because of a bug. Maybe it's been fixed now? (I can do this in another PR)

the getting_started.md page is called "running your first simulation" yet doesn't have code to run a simulation (also for another PR maybe - I thought Julia had a PR for it but I can't find it anymore)

The file structure in the code doesn't match the menu structure of the docs. it makes it a bit hard to navigate.
docs menu:
image
docs code:
image

The page changing_canopy_parameterizations is empty
image

Maybe we don't want to show the output (or silence warning) of some box of code, e.g., running fluxnet simulations:
image

Note this seems quite important - some box of warnings are very long... (if needed I can help with this)

The driver_tutorial page has a long return box that should be hidden
image

Can we rename "Leaderboard" to "Benchmark"? Benchmark is a well known modelling exercise, to compare model output to observation. Leaderboard is more an internal CliMA goal of showing we're the best model...

Note: I see that Kevin added warning boxes in the calibration docs. I think this is pretty neat and we should use it throughout the docs

image

APIs/shared_utilities, simulations, has a blank name (-) (which should be "LandSimulations" I believe)
image

APIs/Soil_Albedo rendering is bugged. It seems to be full of titles (hashtags)
image

"Shared utilities" => "shared_utilities.md",
],
"Physical units" => "physical_units.md",
"Julia background" => "julia.md",
"APIs" => apis,
Copy link
Member

Choose a reason for hiding this comment

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

APIs currently has two nested ClimaLand
image

we should remove the ClimaLand fold (there's only one fold)

Copy link
Member

@imreddyTeja imreddyTeja left a comment

Choose a reason for hiding this comment

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

The new make_set_fluxnet_initial_conditions is much cleaner than defining it is the simulation script!

I left a few comments, mostly about the assumption that start_date = t0.epoch (which is almost always the case).

@kmdeck kmdeck force-pushed the kd/fluxnet_documentation branch from 3c6b2c6 to 67625e7 Compare August 13, 2025 19:10
@kmdeck kmdeck force-pushed the kd/fluxnet_documentation branch from 67625e7 to ed30c1a Compare August 13, 2025 20:24
@kmdeck kmdeck mentioned this pull request Aug 13, 2025
1 task
@juliasloan25
Copy link
Member

I had the same HTTP error in the ubuntu tests - I retried and it ran ok

@kmdeck kmdeck enabled auto-merge (squash) August 13, 2025 21:57
@kmdeck kmdeck merged commit 0a873e3 into main Aug 13, 2025
20 of 23 checks passed
@kmdeck kmdeck deleted the kd/fluxnet_documentation branch August 13, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants