Skip to content

feat(benchmark): Phase 4 validation pipeline integration, metric_CRPS… - #4062

Open
ayushman1210 wants to merge 13 commits into
PecanProject:developfrom
ayushman1210:phase4-benchmarking-integration
Open

feat(benchmark): Phase 4 validation pipeline integration, metric_CRPS…#4062
ayushman1210 wants to merge 13 commits into
PecanProject:developfrom
ayushman1210:phase4-benchmarking-integration

Conversation

@ayushman1210

Copy link
Copy Markdown
Contributor

Description

This PR implements Phase 4: Biogeochemistry MVP Integration and Initial Benchmarks (#4059), proving the decoupled PEcAn validation pipeline end-to-end against real model output and observations.

Key changes include:

  • metric_CRPS.R Implementation & Metric Registry: Added Continuous Ranked Probability Score in metric_CRPS.R using scoringRules::crps_sample() with pure-R analytical fallback. Registered CRPS in pecan_metric_registry and added unit test suite test-metric_CRPS.R (6/6 PASS).
  • Salinas SOCs Quarto HTML Scorecard: Created salinas-socs/ensemble_benchmark/run_benchmarks.R to ingest EFI-long ensemble outputs, align monthly model data to annual observations with align_by_time(), compute point metrics (RMSE, MAE, R2) & spread metrics (Coverage, CRPS), and compile a dynamic HTML report (Salinas_SOC_Validation_Report.html).
  • Fast Deterministic CI Integration Test: Added test-ensemble_integration.R in testthat (18/18 PASS) to verify time alignment, metric calculations, and scorecard column structures without heavy model execution in CI.
  • Visualization & Coverage Enhancements: Refactored metric_Coverage.R for direct in-band scoring and updated metric_timeseries_plot.R with numerical PMU scores and Status: PASS/FAIL annotations.

Motivation and Context

This change fulfills Phase 4 of the GSoC 2026 Validation Toolkit workplan. Building on the core data intake API (#4017), test coverage (#4032), and Quarto reporting layer (#4040, #4041), Phase 4 proves the pipeline on real biogeochemistry datasets (Salinas SOC ensemble & AmeriFlux MVP) and introduces ensemble spread metrics (CRPS, Coverage).

Closes #4059.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ayushman1210
ayushman1210 force-pushed the phase4-benchmarking-integration branch from 6bb6f9a to 25e7854 Compare August 4, 2026 07:11
@ayushman1210 ayushman1210 moved this from In Progress to Review in GSOC Benchmarking and Validation Aug 15, 2026

@divine7022 divine7022 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this looks good to me. I left few inline comments, but I haven’t finished my review yet

Comment thread modules/benchmark/R/metric_Coverage.R Outdated
Comment thread modules/benchmark/R/metric_timeseries_plot.R Outdated
Comment thread modules/benchmark/R/metric_timeseries_plot.R Outdated
Comment thread modules/benchmark/R/metric_residual_plot.R Outdated
Comment thread modules/benchmark/R/metric_scatter_plot.R
Comment thread modules/benchmark/R/metric_CRPS.R
Comment thread salinas-socs/ensemble_benchmark/README.md Outdated
@divine7022

Copy link
Copy Markdown
Member

metric work here looks good
but on dir structure I would rather not add salinas-socs/ as a new top level directory

some relevant places are modules/benchmark/inst/
fyi, if we think this as kinda pipeline rather than package example workflows/ is the other option. either of those is somewhere I think to look; a tree of its own isn't . I defer this to @dlebauer for thoughts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would like to slow down on committing this one. this done as a part of magic; so the question whether data can live here ? then comes a question, It's that this file arrived with no citation and no derivation, and once it's in the tree it falls under LICENSE -- I defer this @dlebauer for clarification

@dlebauer dlebauer Aug 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it would make more sense to put the example run in a package vignette (probably best).

Regarding including this dataset, I dont think it makes sense to include a dataset here. And, you make a good point about the license, but I think it is sufficient to cite the source of the data in the README.md.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The vignette can use a small fixture: perhaps two systems, two years, and 10 ensemble members.

Comment thread modules/benchmark/R/metric_CRPS.R
Comment thread salinas-socs/ensemble_benchmark/run_benchmarks.R Outdated
Comment thread salinas-socs/ensemble_benchmark/run_benchmarks.R Outdated
Comment thread salinas-socs/ensemble_benchmark/README.md Outdated
Comment thread salinas-socs/ensemble_benchmark/run_benchmarks.R Outdated
Comment thread salinas-socs/ensemble_benchmark/run_benchmarks.R Outdated
Comment thread salinas-socs/ensemble_benchmark/run_benchmarks.R Outdated
Comment thread salinas-socs/ensemble_benchmark/run_benchmarks.R Outdated
@dlebauer dlebauer added the ccmmf issues and pre related to the ccmmf project label Aug 24, 2026
@dlebauer
dlebauer self-requested a review August 25, 2026 00:52

@dlebauer dlebauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @ayushman1210 , this is great progress. I have a number of in-line comments.

A few key points:

  1. we should not add a new salinas-socs directory in the PEcAn root. this could go under examples/ and / or it could be included as a package vignette.

  2. PR description claims that it closes #4059, but it doesn't appear to meet (or explain deviations from) specifications in that issue. For example:

  • there is no Ameriflux dataset
  • bias metric (it is calculated for the plot, but there is no metric_Bias function)
  • there is no spaghetti plot
  1. Please revise README for clarity, style, and consistency with current state. Style: use standard capitalization and concise, complete sentences, particularly in the introduction, file descriptions, and workflow steps. Distinguish committed files from generated or external inputs—ensemble_output.csv and build_salinas_soc_obs.py are referenced but not included. Explain how the monthly model output should be compared with the annual SOC observations.

Comment thread salinas-socs/ensemble_benchmark/README.md Outdated
Comment thread examples/salinas_soc_ensemble/run_benchmarks.R
Comment thread examples/salinas_soc_ensemble/run_benchmarks.R

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The vignette can use a small fixture: perhaps two systems, two years, and 10 ensemble members.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ccmmf issues and pre related to the ccmmf project dockerfile modules tests

Projects

Status: Review

Development

Successfully merging this pull request may close these issues.

Phase 4: Biogeochemistry MVP Integration and Initial Benchmarks

4 participants