Mitokic/09042026/best model updates - #278
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Default recovery can abort before forecasting when an expected best-run artifact is absent for RDS or Parquet runs.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Enhances forecast selection with plausibility checks, robust recovery, and quality-aware Agent workflows.
Changes:
- Adds trend, seasonality, validity, and accuracy-aware model selection.
- Improves hierarchical reconciliation, updates, retries, and artifact recovery.
- Expands tests, documentation, benchmarks, and package metadata.
File summaries
| File | Description |
|---|---|
R/agent_iterate_forecast.R |
Integrates selection quality into Agent iterations and logging. |
R/agent_run.R |
Prevents retries for forecast-selection rejections. |
R/agent_update_forecast.R |
Adds update quality assessment and recovery. |
R/ensemble_models.R |
Screens ensemble inputs. |
R/final_models.R |
Implements quality-aware final selection and restart handling. |
R/forecast_time_series.R |
Documents selection behavior. |
R/hierarchy.R |
Validates reconciliation inputs. |
R/prep_data.R |
Persists original-target differencing values. |
R/read_write_data.R |
Uses exact reconciled artifact reads locally. |
R/train_models.R |
Preserves invalid predictions and reconstructs original targets. |
tests/testthat/test-agent-selection-policy.R |
Tests Agent selection and logging policy. |
tests/testthat/test-agent-update-selection.R |
Tests update-time selection behavior. |
tests/testthat/test-artifact-restarts.R |
Tests completed hierarchical restarts. |
tests/testthat/test-best_models.R |
Removes relocated shared fixtures. |
tests/testthat/test-final-models-restart.R |
Tests selection restart recovery. |
tests/testthat/test-forecast-selection-corners.R |
Covers selection edge cases. |
tests/testthat/test-forecast-selection-trend.R |
Covers trend-aware selection. |
tests/testthat/test-multistep-daily-regression.R |
Bounds expensive model parameters in tests. |
tests/testthat/test-reconciled-forecast-selection.R |
Tests pre-reconciliation selection. |
tests/testthat/test-undifference.R |
Tests distinct original-target reconstruction. |
tools/benchmark-forecast-selection.R |
Adds selection and hierarchy benchmarks. |
tools/validate-agent-guidance.R |
Validates package and development exclusions. |
vignettes/ai-agent.Rmd |
Documents Agent quality behavior. |
vignettes/best-model-selection.Rmd |
Documents selection rules and limitations. |
man/final_models.Rd |
Updates generated final-model documentation. |
man/forecast_time_series.Rd |
Updates generated forecasting documentation. |
man/iterate_forecast.Rd |
Updates generated iteration documentation. |
man/update_forecast.Rd |
Updates generated update documentation. |
NEWS.md |
Records the selection improvements. |
DESCRIPTION |
Bumps the development version. |
.Rbuildignore |
Excludes additional Agent tooling directories. |
Review details
Files not reviewed (4)
- man/final_models.Rd: Generated file
- man/forecast_time_series.Rd: Generated file
- man/iterate_forecast.Rd: Generated file
- man/update_forecast.Rd: Generated file
- Files reviewed: 31/35 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
Restart validation, optional remote artifacts, backend consistency, and logged accuracy still have correctness issues.
Get a fresh assessment by requesting another Copilot review.
Review details
Files not reviewed (4)
- man/final_models.Rd: Generated file
- man/forecast_time_series.Rd: Generated file
- man/iterate_forecast.Rd: Generated file
- man/update_forecast.Rd: Generated file
Suppressed comments (1)
R/agent_update_forecast.R:1432
- This checkpoint is optional for a genuinely new series, but the unconditional exact read only happens to degrade a missing CSV into an empty table. With local RDS/Parquet (and typical remote exact reads), a nonexistent
-agent_best_runartifact raises beforesubmit_fcst_run()runs, so new-series recovery fails for supported output backends. Use an optional exact read that treats only not-found as empty while still propagating authentication and other storage errors.
agent_best_run_tbl <- read_file(agent_info_lean$project_info,
- Files reviewed: 36/40 changed files
- Comments generated: 4
- Review effort level: Balanced
FinnTS Model Selection: Branch WalkthroughThe main change is:
1. Use Comparable EvidenceEvery candidate is checked against the same expected backtest and future dates, using historical actuals restored to their original scale.
2. Reject Invalid CandidatesThese are hard failures: the candidate cannot win or contribute to a simple average.
3. Build And Assess AveragesFinn screens existing individual and learned-ensemble outputs, then forms the requested simple averages from hard-eligible candidates, up to 4. Check Future PlausibilityThese are generally soft concerns, not automatic disqualifications.
5. Choose Among Close ContendersAfter hard failures are removed, the shortlist includes candidates within best WMAPE + the larger of 0.5 percentage points or 5% of best WMAPE.
6. Reconcile After SelectionFor hierarchical forecasts, each hierarchy node can choose a different model or average. Finn reconciles that selected mixture using the existing 7. Keep Agent Iteration Decisions SeparateThe Agent’s existing 10% rule is preserved, not replaced by the new shortlist allowance:
8. Apply Stricter Rules To Updates
9. Recover Saved Results SafelyA filename, completion log, or Bottom LineThe intent is better-informed selection, not guaranteed future accuracy. Real regime changes, weak history, and reconciliation can still produce questionable outcomes. Plausibility helps choose among sufficiently accurate candidates; it does not guarantee that a good candidate exists. |
| @@ -0,0 +1,1138 @@ | |||
| normalize_series_history <- function(data, hist_end_date, recipe = "R1", | |||
There was a problem hiding this comment.
I think it would be helpful to add comments to some of the functions in this file of what it does and implemented in certain way, to improve readability and maintainability
This pull request introduces significant improvements to the model selection and forecast validation process, focusing on balancing backtest accuracy with forecast plausibility and robust handling of rejected or incomplete forecasts. The changes enhance the reliability of model selection, improve run recovery, and ensure that only valid, high-quality forecasts are accepted and logged. Additionally, the pull request refactors how forecast files are loaded and validated, and updates documentation and versioning to reflect these improvements.
Model Selection and Quality Evaluation Enhancements:
NEWS.md,R/agent_iterate_forecast.R) [1] [2]R/agent_iterate_forecast.R) [1] [2] [3] [4] [5]Forecast File Handling and Validation:
R/agent_iterate_forecast.R) [1] [2] [3] [4] [5] [6]R/agent_iterate_forecast.R)Run Logging and Recovery Improvements:
R/agent_iterate_forecast.R) [1] [2]R/agent_iterate_forecast.R) [1] [2]Documentation and Versioning:
0.7.0.9006and documented the improvements inDESCRIPTIONandNEWS.md. (DESCRIPTION,NEWS.md) [1] [2].Rbuildignoreto exclude new agent-related files. (.Rbuildignore)These changes collectively improve the framework’s ability to select, validate, and recover high-quality forecasts while maintaining clear logging and robust error handling.