feat: add permutation feature importance to JointFMClient - #12
Open
mariusvilkas wants to merge 3 commits into
Open
feat: add permutation feature importance to JointFMClient#12mariusvilkas wants to merge 3 commits into
mariusvilkas wants to merge 3 commits into
Conversation
Adds JointFMClient.feature_importance(), which permutes each feature column
across history rows and compares the resulting forecast against a shared
baseline forecast_samples() call. Returns one dict per feature, keyed by
metric ("mean" absolute shift, "distance" centered squared 2-Wasserstein)
then target then horizon, so callers get both scores across every
target/horizon combination without flattening rows themselves.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
mariusvilkas
requested review from
DRMetaplectic,
j1z0 and
shackmann
as code owners
August 28, 2026 16:58
The presence check only looked at rows[0], then row[feature] was read unconditionally on every row, so a sparse/nullable history row that omits the permuted column later in the sequence raised KeyError even though forecast() already accepts that shape (it only requires a declared column to appear in some row, not every row). Now presence is checked across all rows, and only rows where the key is present are shuffled; rows without it are left untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 62dff02. Configure here.
forecast() always takes the row-payload path when schema is set, even for DataFrame history, so passing a DataFrame together with a DataFrameSchema (a combination permute_history_column already supports) raised "history_rows must be a JSON array" before any permutation ran. _sample_forecast_for_importance now converts the frame to rows itself via dataframe_to_history_rows in that case, so callers who already have a schema and a frame don't have to convert rows by hand. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

Adds JointFMClient.feature_importance(), which permutes each feature column across history rows and compares the resulting forecast against a shared baseline forecast_samples() call. Returns one dict per feature, keyed by metric ("mean" absolute shift, "distance" centered squared 2-Wasserstein) then target then horizon, so callers get both scores across every target/horizon combination without flattening rows themselves.
I validated locally with script
On response got raw (not normalized values)
Note
Low Risk
Additive SDK surface on the existing forecast path with unit tests; main operational note is extra prediction calls (one baseline plus one per feature column).
Overview
Adds
JointFMClient.feature_importance(...), a permutation-based importance workflow on top of sample forecasts. The client runs one shared baselineforecast(..., return_mode="samples"), then for each listed feature shuffles that column across history rows (marginal preserved) and re-forecasts with the sameseedandn_samples.Results are one dict per feature with
mean(absolute shift in forecast mean) anddistance(centered squared 2-Wasserstein between baseline and permuted samples), nested by target and horizon label.horizonsmust align positionally withquery_times. DataFrame history plus an explicitschemais converted to rows before forecasting so it matches the row-payload path.New
jointfm_client.feature_importancehelpers implement column permutation (row dicts or pandas),sample_w2_distance, and scoring assembly. API reference documents the method; tests cover end-to-end client behavior, validation errors, and helper edge cases.Reviewed by Cursor Bugbot for commit 3d9e02d. Bugbot is set up for automated code reviews on this repo. Configure here.