Skip to content

Refactor the baseline Validation Pipeline to align with GSoC Architecture#4017

Merged
dlebauer merged 35 commits into
PecanProject:developfrom
ayushman1210:gsoc/phase2-architecture
Jul 2, 2026
Merged

Refactor the baseline Validation Pipeline to align with GSoC Architecture#4017
dlebauer merged 35 commits into
PecanProject:developfrom
ayushman1210:gsoc/phase2-architecture

Conversation

@ayushman1210

@ayushman1210 ayushman1210 commented May 31, 2026

Copy link
Copy Markdown
Contributor

This issue tracks the follow-up changes required for the dataframe-first validation framework introduced in PR #3892 aligning it with the architecture outlined in my GSoC 2026 workplan

Planned Changes

  1. Modular 4-Stage Pipeline: Refactor the monolithic run_benchmark() entry point into a flexible 4-stage pipeline: Validate -> Align -> Compute -> Plot.
  2. Base R / dplyr Integration: Replace the existing naive alignment logic with robust base R operations (exp findInterval) and dplyr for spatial and temporal alignment, ensuring high performance while removing external dependencies
  3. BETYdb Decoupling: Ensure that the pipeline is completely independent of BETYdb IDs, relying solely on standard variable names and tabular data structures.
  4. Generalized Metrics: Expand the metric computation (RMSE, MAE, R2, correlation) to ensure they operate cleanly on the aligned data frames, independent of legacy database structures.

Context

PR #3892 established the baseline dataframe-first concept. The changes tracked in this issue will evolve it into the generalized, high-performance toolkit planned for Phase 2 of my GSoC project.

@divine7022 divine7022 self-requested a review June 1, 2026 17:42
@infotroph

Copy link
Copy Markdown
Member

data.table Integration: Replace the existing naive alignment logic with high-performance data.table operations

Please convert this to a dplyr or base R approach. data.table is a good package that lots of people use happily, but it is not widely used in PEcAn and its syntax is confusing to folks who don't know it already.

@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.

overall it's in good shape, I dropped inline for thing I'd want a look at before merge

Comment thread modules/benchmark/R/run_benchmark.R Outdated
Comment thread modules/benchmark/README.md
Comment thread modules/benchmark/R/data_intake.R
Comment thread modules/benchmark/R/run_benchmark.R
Comment thread modules/benchmark/R/run_benchmark.R Outdated
Comment thread modules/benchmark/R/run_benchmark.R Outdated
Comment thread modules/benchmark/tests/testthat/test-run_benchmark.R
Comment thread modules/benchmark/R/run_benchmark.R
Comment thread modules/benchmark/R/run_benchmark.R
ayushman1210 and others added 2 commits June 22, 2026 18:38
- Refactor run_benchmark to use 'obvs' naming convention and point plots
- Update bm_validate to explicitly check for missing time/value columns
- Replace stop() and warning() with PEcAn.logger equivalents
- Abstract METRIC_REGISTRY into an extensible package environment
- Add NSE metric and MEF alias
- Update README to reflect correct data.frame inputs
- Manually export load_and_map_data and add man page
- Add test coverage for missing columns, R2 metric, and tolerance drops
- Add info log for time alignment tolerance dropping
@ayushman1210 ayushman1210 requested a review from divine7022 June 23, 2026 12:16

@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.

LGTM! thanks
heads up, never hand edit the NAMESPACE and man pages, they are supposed to be auto generated

@dlebauer dlebauer added this pull request to the merge queue Jul 2, 2026
Merged via the queue into PecanProject:develop with commit 67dded7 Jul 2, 2026
19 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

5 participants