Skip to content

ENH: Implement efficient _evaluate_by_index for RelativeLoss (#4304)#9400

Open
Vbhatt03 wants to merge 1 commit intosktime:mainfrom
Vbhatt03:enhance/relative-loss-evaluate-by-index
Open

ENH: Implement efficient _evaluate_by_index for RelativeLoss (#4304)#9400
Vbhatt03 wants to merge 1 commit intosktime:mainfrom
Vbhatt03:enhance/relative-loss-evaluate-by-index

Conversation

@Vbhatt03
Copy link
Copy Markdown

Reference Issues/PRs

Fixes #4304

What does this implement/fix? Explain your changes.

This PR implements the efficient _evaluate_by_index method for the RelativeLoss forecast performance metric, replacing the computationally expensive jackknife pseudo-sampling approach with direct point-wise evaluation.

Changes:

  • Added _evaluate_by_index method to RelativeLoss class that:

    • Validates that y_pred_benchmark parameter is provided
    • Creates instances of the underlying loss function with multioutput="raw_values" for point-wise evaluation
    • Computes loss for both predictions and benchmark predictions at each time point
    • Calculates the ratio point-wise: loss_pred / max(loss_benchmark, EPS) where EPS prevents division by zero
    • Properly aggregates results based on the multioutput parameter
  • Added necessary imports:

    • numpy for array operations
    • EPS constant from sktime.performance_metrics.forecasting._functions

Key improvements:

  • Significantly more efficient: $O(n)$ direct computation vs. $O(n^2)$ jackknife approach
  • Maintains full compatibility with existing API
  • Properly handles multioutput aggregation (raw_values vs uniform_average)

Does your contribution introduce a new dependency?

No new dependencies. Uses only existing imports: numpy and EPS from internal functions module.

What should a reviewer concentrate their feedback on?

  • Correctness of the point-wise ratio calculation
  • Proper handling of the multioutput parameter and variable aggregation
  • Validation that y_pred_benchmark is required and handled appropriately
  • Consistency with the pattern used in similar metrics (e.g., MeanAbsoluteScaledError)

Did you add any tests for the change?

The implementation uses existing test infrastructure passed by RelativeLoss. All 224 performance metrics tests pass successfully, validating:

  • Correctness of _evaluate_by_index implementation
  • Proper error handling for missing y_pred_benchmark
  • Correct multioutput aggregation
  • Consistency with the aggregate _evaluate method

PR checklist

  • The PR title starts with [ENH] - this is an enhancement improving code efficiency
  • No new dependencies introduced
  • All existing tests pass (224 tests)
  • Not a new estimator, so API reference and docstring examples checklist items N/A

…4304)

- Add direct point-wise evaluation instead of jackknife pseudo-sampling
- Respects multioutput parameter for proper variable aggregation
- Handles y_pred_benchmark correctly
- All 224 tests passing
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

[ENH] implement efficient _evaluate_by_index for forecast performance metrics

2 participants