add ca fertilization harmonization and ncc compost support#4002
Open
divine7022 wants to merge 36 commits into
Open
add ca fertilization harmonization and ncc compost support#4002divine7022 wants to merge 36 commits into
divine7022 wants to merge 36 commits into
Conversation
14 tasks
dlebauer
reviewed
May 20, 2026
Member
dlebauer
left a comment
dlebauer
left a comment
There was a problem hiding this comment.
Posting a few comments as a partial review.
| #' @format A tibble with one row: | ||
| #' \describe{ | ||
| #' \item{a, b}{Lower and upper truncation bounds.} | ||
| #' \item{mean, sd}{Untruncated mean and standard deviation.} |
Member
There was a problem hiding this comment.
Suggested change
| #' \item{mean, sd}{Untruncated mean and standard deviation.} | |
| #' \item{mean, sd}{Mean and standard deviation of the underlying normal | |
| #' distribution.} |
| # source field so the provenance travels with the data when the package | ||
| # loads. | ||
|
|
||
| # %C distribution. Bernard et al. 2023 (Front. Env. Sci., CA central coast |
Member
There was a problem hiding this comment.
where are the full citations? at a minimum, please include [author, year, DOI] where available or [author, year, title, url] if no DOI.
A few questions come up:
- Using
truncnormfor compost %C and %N distributions? The truncated normal imposes hard discontinuities ataandb. For %C and %N, a Beta might be easier to justify because it is supported in the range [0,1]. Then, for the C:N ratios, one option is to sample from the Betas and calculate the ratio. Another is to directly set a Gamma prior on C:N. - Should these opinionated priors be bundled into the data.land package? I don't have a strong opinion but note that these are distinct from the reference datasets that we usually include to support specific functions. Other options include keeping these in a CCMMF repository, putting these in betydb priors table, or computing these distributions on the fly from the actual numbers in the underlying datasets so that all assumptions are clear.
Comment on lines
+13
to
+14
| "/projectnb/dietzelab/ccmmf/usr/akash/management/fertilization", | ||
| "CCMMF Fertilization - Compost.tsv" |
Member
There was a problem hiding this comment.
avoid
- hard coded paths
- file names with spaces
| #' oz_per_tree_to_lb_per_acre(c(1, 3), tpa = 145) | ||
| #' | ||
| #' @export | ||
| oz_per_tree_to_lb_per_acre <- function(oz_per_tree, tpa) { |
Member
There was a problem hiding this comment.
🤯 plz use PEcAn.utils::ud_convert for such transformations! Hard coded conversions are error prone.
Also, I would suggest quarantining imperial units and not supporting them in PEcAn; i.e. the conversion can be done within the data ingest functions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
folds standalone N fertilization harmonization (was living at /projectnb/dietzelab/ccmmf/management/fertilization/harmonization.R) into data.land data-raw pipeline, then layers ncc (compost) sampling support on top and extends the events schema with an ncc event type
events_schema_v0.1.2.json picks up ncc as an allowed event_type with material, ncc_subtype, fert_subtype, and pft properties
separate workflow PR will consume these samplers from workflows/fertilization-statewide and workflows/ncc-statewide to emit ensemble events
cc @sarahkanee @mdietze @infotroph @dlebauer
Motivation and Context
Review Time Estimate
Types of changes
Checklist: