fix(cpp): restore working feature_map bar rendering (SHAP alignment, sign, labels) + tests#334
Merged
Merged
Conversation
The per-subcategory SHAP impact bars were misaligned: they piled onto the top rows instead of their real subcategory rows. Root cause was commit 0567aba ('redesign feature_map SHAP bars as per-feature cumulative stacks'), which replaced the working implementation -- one value per category in row order, drawn with a single ax.barh(list_cat, values) call -- with a per-feature ax.barh(<category name>, ...) loop that only drew categories with impact, letting matplotlib's categorical converter compact them onto the top rows. Revert the SHAP bar rendering (and its x-limit) to that proven, aligned implementation. Add two regression tests that would catch this class of bug early: a single-category impact must land on its own row, and several impacted categories must map one-to-one to their rows. Full cpp_plot suite green (336). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
breimanntools
force-pushed
the
fix/feature-map-shap-bar-order
branch
from
94152a0 to
a168a2c
Compare
breimanntools
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #334 +/- ##
=======================================
Coverage 94.93% 94.94%
=======================================
Files 185 185
Lines 17883 17868 -15
Branches 3038 3028 -10
=======================================
- Hits 16978 16965 -13
+ Misses 598 597 -1
+ Partials 307 306 -1
... and 1 file with indirect coverage changes
🚀 New features to boost your workflow:
|
… weak sign test Completes the SHAP-bar revert: the top per-position impact bars were left in the 0567aba magnitude-stack (one-direction, abs(v)) while the right subcategory bars are signed-diverging, so the two panels disagreed (a net-negative position read as an inflated upward bar). Revert the top bars to the working signed implementation (pos up / neg down) to match. Rewrite test_shap_bars_are_one_direction -> test_shap_bars_are_signed_diverging: the old test asserted get_x()/get_y() >= 0, which is always true for bar/barh (the signed extent lives in get_width()/get_height()), so it caught nothing and its premise was wrong post-revert. Now asserts both panels diverge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… by heatmap) d59c4bb moved the cumulative-importance % labels inside the bars (white, right-anchored) and dropped the clip_on=False that 8465896 had added, so short-bar labels ran left past the x=0 baseline and were clipped/hidden under the heatmap (e.g. '2.0%' -> '0%'). Restore the never-cut form: label just outside the bar tip (ha='left', extends right, away from the heatmap) with clip_on=False. Add a regression test that no importance label extends left of the baseline. (The ranking plot already handles this correctly.) cpp_plot green (337). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
breimanntools
changed the title
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
1 participant
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.
Restores several
feature_maprendering behaviors that recent commits silently broke, and adds the regression tests that were missing. Traced withgit log -S; each is reverted to the proven implementation rather than re-patched.Fixes
0567abaa). Bars were drawn by category name, only for categories with impact, so matplotlib's categorical converter compacted them onto the top rows. Reverted to one value per category in row order + a singleax.barh(list_cat, values)(row-aligned).0567abaa). They were a one-direction magnitude stack while the right bars are signed; a net-negative position read as an inflated upward bar. Reverted to the signed-diverging form (pos up / neg down) to match.d59c4bb9). The labels were moved inside the bars (white, right-anchored) and lost8465896b'sclip_on=False, so short-bar labels ran left past the baseline and were hidden (e.g.2.0%→0%). Restored the never-cut form: label just outside the bar tip (extends right),clip_on=False.Tests (this class now fails fast)
test_shap_impact_bar_aligns_with_correct_row,test_shap_impact_bars_map_one_to_one_to_rows— SHAP bars map 1:1 to rows.test_shap_bars_are_signed_diverging(rewritten from a no-op that checkedget_x()>=0) — both panels diverge in sign.test_importance_bar_labels_extend_outward_not_clipped— no % label extends left of the baseline.Full
cpp_plotsuite green (337). Only feature_map bar/label rendering is touched; independent of the auto_font / prediction PRs.Known, NOT changed here (deliberate tuned layout; only affects explicit non-square figsizes, not the default or auto_font): the scale-category legend can sit off-figure for e.g.
figsize=(12,6). Left for a separate decision.🤖 Generated with Claude Code