Skip to content

bug fix - #291

Merged
Mike Tokic (mitokic) merged 1 commit into
mainfrom
mitokic/10152026/back-test-target-issue
Oct 15, 2026
Merged

Mike Tokic (mitokic) merged 1 commit into
mainfrom
mitokic/10152026/back-test-target-issue

Conversation

@mitokic

Copy link
Copy Markdown
Collaborator

This pull request updates the Croston model handling to ensure that historical actuals are preserved in backtests, CSV outputs, and model averages, while continuing to clamp negative targets only during Croston training. It also adds comprehensive tests to validate this behavior and updates documentation accordingly.

Bug Fixes and Improvements for Croston Model:

  • Croston now preserves historical actuals in backtests, downstream CSV outputs, and model averages; negative targets are still clamped to zero for Croston training only.
  • The clamp_negative_target function now saves the original Target values in a Target_Original column (if not already present) before clamping, ensuring that assessment actuals remain unchanged and only training data is affected.

Testing Enhancements:

  • Added tests to verify that Croston clamping preserves assessment actuals, does not alter training data outside of training splits, and that CSV/model outputs retain historical actuals. Also checks that the clamping is a no-op for other models and when no negatives exist. [1] [2]

Documentation and Metadata Updates:

  • Updated the package version to 0.7.0.9013 in DESCRIPTION and NEWS.md to reflect these changes. [1] [2]

Copilot AI left a comment

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.

🔵 Needs a closer look

Validation remains unverified; devtools::test(filter = "clamp_negative_target") and devtools::check() need confirmation before approval.

0 open findings

What changed in this PR

Preserves Croston historical actuals throughout FinnTS backtests and downstream outputs while retaining non-negative training targets.

Changes:

  • Saves original targets before Croston clamping.
  • Adds regression coverage for assessment actuals, CSV outputs, and model averages.
  • Updates release notes and package version to 0.7.0.9013.
File Description
tests/​testthat/​test-clamp_negative_target.R Tests actual preservation and downstream outputs.
R/​train_models.R Preserves original targets before clamping.
NEWS.md Documents the Croston correction.
DESCRIPTION Advances the development version.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mitokic
Mike Tokic (mitokic) merged commit ad73100 into main Oct 15, 2026
10 of 14 checks passed
@mitokic
Mike Tokic (mitokic) deleted the mitokic/10152026/back-test-target-issue branch October 15, 2026 22:31
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