fix: detect forecaster-chain as invalid in sequential pipeline validation#457
Open
kpal002 wants to merge 1 commit into
Open
fix: detect forecaster-chain as invalid in sequential pipeline validation#457kpal002 wants to merge 1 commit into
kpal002 wants to merge 1 commit into
Conversation
…tion The ENSEMBLE composition rule (forecasting → forecasting, position="any") was matching during validate_pipeline(), causing the explicit forecaster-chain error to be silently skipped. Two forecasters in a sequential pipeline are always invalid — ENSEMBLE composition is parallel, not sequential. Fixes: forecaster → forecaster pair returns Valid: True in Step 5 of examples/01_forecasting_workflow.py when it should return Valid: False.
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
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.
Reference Issues/PRs
Fixes #445.
What does this implement/fix? Explain your changes.
Root cause:
COMPOSITION_RULEScontains an ENSEMBLE rule (forecasting → forecasting,position="any") which matched theNaiveForecaster → ExponentialSmoothingpair during sequential pipeline validation. Because an applicable rule was found, the explicit forecaster-chain error check in_check_pair_compatibilitywas never reached, causing the validator to returnvalid=True.Fix in
src/sktime_mcp/composition/validator.py:Added an early-return guard in
_check_pair_compatibilityforforecasting → forecastingpairs before the rule lookup. Two forecasters in a sequential pipeline are always invalid — ENSEMBLE composition is parallel, not sequential.Added
and rule.composition_type != CompositionType.ENSEMBLEto the rule-matching condition to make it explicit that ENSEMBLE-type rules should not be used to approve sequential pipeline pairs.Result:
validate_pipeline(["NaiveForecaster", "ExponentialSmoothing"])now correctly returnsvalid=Falsewith the error:This matches the
❌icon and# Invalid pipeline: Forecaster -> Forecastercomment inexamples/01_forecasting_workflow.pyStep 5.Does your contribution introduce a new dependency? If yes, which one?
No.
What should a reviewer concentrate their feedback on?
_check_pair_compatibility(lines 262–271) — confirm that sequential forecaster chaining should always be invalid with no exceptions.CompositionType.ENSEMBLEfilter in the rule lookup — confirm that ENSEMBLE rules should never apply in a sequential pipeline context.Any other comments?
No new dependencies. No API changes. The fix is purely internal to
_check_pair_compatibility.PR checklist
For all contributions