Skip to content

Notebooks for AGU 2025 sims#17

Open
infotroph wants to merge 23 commits intomainfrom
agu-2025
Open

Notebooks for AGU 2025 sims#17
infotroph wants to merge 23 commits intomainfrom
agu-2025

Conversation

@infotroph
Copy link
Contributor

No description provided.

@dlebauer dlebauer requested a review from divine7022 February 19, 2026 16:25
Comment on lines +81 to +88
function(site_id, start_date, end_date, ens_id, ...) {
PEcAn.SIPNET::met2model.SIPNET(
in.path = file.path(
args$site_era5_path,
paste("ERA5", site_id, ens_id, sep = "_")
),
start_date = args$start_date,
end_date = args$end_date,
Copy link
Contributor

Choose a reason for hiding this comment

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

function signature binds start_date and end_date from file_info, but lines 87-88 ignore them and use args$start_date / args$end_date instead; this works but is misleading, consider using the function parameter or removing it from the signature. (same pattern exists in 2a_grass/01_ERA5_nc_to_clim.R was this intentionally inherited?)

Comment on lines +64 to +68
# start from scratch if no continue is passed in
status_file <- file.path(settings$outdir, "STATUS")
if (args$continue && file.exists(status_file)) {
file.remove(status_file)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

comment says "start from scratch if no continue is passed" but the code removes STATUS when continue=TRUE; which means --continue causes a restart. This looks opposite, should the condition be !args$continue?
is this a pre-existing bug being carried forward?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is copied straight from the venerable/long-in-tooth web/workflow.R, where "continue" meant "attempt to come back to a workflow that errored out before" and never worked as intended. It's probably time to just delete the whole block from this file.

export NCPUS=8
ln -s [your/path/to]/sipnet/sipnet sipnet.git
[host_args] ./05_run_model.R --settings=val_out/pecan.CONFIGS.xml

Copy link
Contributor

Choose a reason for hiding this comment

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

missing closing ```for the code block
5 and 6 will render incorrectly

```{sh}
[host_args] ./validate.R \
--model_dir=val_out \
--output_dir=validation_results_$(date'+%s')
Copy link
Contributor

Choose a reason for hiding this comment

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

missing space shell will try to find a command called date'+%s'

Suggested change
--output_dir=validation_results_$(date'+%s')
--output_dir=validation_results_$(date '+%s')

facet_wrap(~site) +
theme_bw() +
ylab("LAI") +
xlab("DOY") +
Copy link
Contributor

Choose a reason for hiding this comment

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

trailing + on the last ggplot layer save_gg(...) will be parsed as a ggplot layer addition and throw an error

Suggested change
xlab("DOY") +
xlab("DOY")

# for any CA location(s) in site_info

set.seed(6824625)
library(tidyverse)
Copy link
Contributor

Choose a reason for hiding this comment

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

would be lighter and more explicit (not blocker, just suggestion)

Suggested change
library(tidyverse)
library(dplyr); library(tidyr); library(purrr)

Comment on lines +39 to +41
`./01_ERA4_nc_to_clim.R --start_date=2016-01-01` on my machine becomes
`sbatch -n16 --mem=12G --mail-type=ALL --uid=jdoe \
./01_ERA4_nc_to_clim.R --start_date=2016-01-01` on yours.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
`./01_ERA4_nc_to_clim.R --start_date=2016-01-01` on my machine becomes
`sbatch -n16 --mem=12G --mail-type=ALL --uid=jdoe \
./01_ERA4_nc_to_clim.R --start_date=2016-01-01` on yours.
`./01_ERA5_nc_to_clim.R --start_date=2016-01-01` on my machine becomes
`sbatch -n16 --mem=12G --mail-type=ALL --uid=jdoe \
./01_ERA5_nc_to_clim.R --start_date=2016-01-01` on yours.

lon = seq(from = -124.5, to = -114, by = 0.5),
lat = seq(from = 32.5, to = 42, by = 0.5)
) |>
mutate(id = paste0(lat, "N_", abs(lon), "W"))
Copy link
Contributor

Choose a reason for hiding this comment

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

need library(dplyr) or dplyr::mutate()

intersect(ca_shp) |>
_$id
ca_grid <- ca_bboxgrid |>
filter(id %in% ca_gridcell_ids)
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

Comment on lines +57 to +59
pft = dplyr::case_when(
crop_class %in% c("G", "F", "P", "T") ~ "grass",
crop_class %in% c("D", "C", "V", "YP") ~ "temperate.deciduous",
Copy link
Contributor

Choose a reason for hiding this comment

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

since this PR introduces an annual_crop pft for the lebauer analysis , should "T" map there instead? also, "G" includes grain crops which are annual, should those also be annual_crop ?

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.

2 participants