Skip to content

Begin phase 3 with HSP validation data#16

Merged
infotroph merged 26 commits intomainfrom
phase-3
Apr 7, 2026
Merged

Begin phase 3 with HSP validation data#16
infotroph merged 26 commits intomainfrom
phase-3

Conversation

@infotroph
Copy link
Copy Markdown
Contributor

Pushing for visibility, details TK

@infotroph infotroph changed the title WIP: Begin phase 3 with HSP validation data Begin phase 3 with HSP validation data Mar 24, 2026
@divine7022 divine7022 self-requested a review March 24, 2026 21:17
@divine7022
Copy link
Copy Markdown
Contributor

PR #17 looks like it includes everything in this PR (#16) plus just a couple extra commits (one_off_analysis) since #17 is already being reviewed, can we just roll with #17 and close #16 so we’re not reviewing the same stuff twice ?

@infotroph
Copy link
Copy Markdown
Contributor Author

@divine7022 Sorry for the duplicates -- #17 is really intended to just be the AGU notebooks and I'd like to get this one merged first so that the diff is comprehensible there. I'll leave breadcrumbs to my changes in this branch that address your comments from #17.

Copy link
Copy Markdown
Contributor

@dlebauer dlebauer left a comment

Choose a reason for hiding this comment

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

Looks good. A few comments / questions, but ready to merge.

Comment thread 3_rowcrop/README.md
Comment thread 3_rowcrop/README.md

Statewide runs continue to use the 198 sites evaluated in phase 2.
We also introduce focused validation runs using the subset of sites where
direct observations of soil carbon and/or biomass are available during the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is biomass validated here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not yet

Comment thread 3_rowcrop/README.md
probably need a "drop into this directory with this format"
step. do NOT include validation_site_info.csv

#### Validation data
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it would be useful to describe what the validation data must contain, and whether there is a common format that we should use that will work across datasets. I'd suggest following the existing naming from PEcAn/BETYdb where applicable.

# TODO is this desirable?
# In production, may be better to complain if no PFT match
drop_na(pft) |>
# Temporary hack:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just for clarity, to make sure i understand: given this was filtered to 'control' and 'none' above: https://github.com/ccmmf/workflows/pull/16/changes#diff-cf89db02c85dc7dfd85dcf4d1a504296836708ed83a2e40b89ab1950f210217cR27

Is the only case of multiple treatments where there are both 'control' and 'none'? does this indicate that there is data cleaning to do?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This deserves a more careful check, but (1) some sites report several measurements separately for the same treatment (these could probably be averaged together if we want), but mostly (2) the motivation here was just "During initial devleopment, only run the model once for a given location until we start passing it treatments that can be expected to differ." Stay tuned.

@infotroph infotroph merged commit c6f01e9 into main Apr 7, 2026
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.

3 participants