Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: generate valid EBMModel when merging #578

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

DerWeh
Copy link
Contributor

@DerWeh DerWeh commented Oct 19, 2024

This PR is meant to fix #576. Upon merging, we average numerical parameters.
I would add a warning to the documentation, that we do not guarantee that EBMs produced by merge_ebms can be fitted. You often allow for so many different types of arguments, that hardly can catch every edge case meaningfully.

@paulbkoch please take a look at _initialize_ebm. Is this how you imagine handling the parameters? I would appreciate some early feedback. If this is in general what you have in mind, I'll try cleaning up the code some more. But it's quite messy, as your API is so flexible.

TODO

  • Cover special cases monotonize and exclude
  • Document limitations of parameter inference

@DerWeh DerWeh marked this pull request as draft October 19, 2024 18:35
Copy link

codecov bot commented Oct 19, 2024

Codecov Report

Attention: Patch coverage is 58.77193% with 47 lines in your changes missing coverage. Please review.

Project coverage is 75.69%. Comparing base (9fc4eb5) to head (d22f52b).
Report is 17 commits behind head on develop.

Files with missing lines Patch % Lines
...erpret-core/interpret/glassbox/_ebm/_merge_ebms.py 58.77% 47 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #578   +/-   ##
========================================
  Coverage    75.68%   75.69%           
========================================
  Files           72       72           
  Lines         9078     9102   +24     
========================================
+ Hits          6871     6890   +19     
- Misses        2207     2212    +5     
Flag Coverage Δ
bdist_linux_310_python 75.26% <58.77%> (-0.09%) ⬇️
bdist_linux_311_python 75.29% <58.77%> (-0.07%) ⬇️
bdist_linux_312_python 75.26% <58.77%> (-0.04%) ⬇️
bdist_linux_39_python 75.21% <58.40%> (-0.15%) ⬇️
bdist_mac_310_python 75.54% <58.77%> (-0.02%) ⬇️
bdist_mac_311_python 75.56% <58.77%> (+0.03%) ⬆️
bdist_mac_312_python 75.52% <58.77%> (-0.04%) ⬇️
bdist_mac_39_python 75.54% <58.40%> (-0.02%) ⬇️
bdist_win_310_python 75.49% <58.77%> (-0.07%) ⬇️
bdist_win_311_python 75.57% <58.77%> (+0.03%) ⬆️
bdist_win_312_python 75.44% <58.77%> (-0.05%) ⬇️
bdist_win_39_python 75.40% <58.40%> (-0.13%) ⬇️
sdist_linux_310_python 75.20% <58.77%> (-0.09%) ⬇️
sdist_linux_311_python 75.30% <58.77%> (+0.01%) ⬆️
sdist_linux_312_python 75.20% <58.77%> (-0.12%) ⬇️
sdist_linux_39_python 75.06% <58.40%> (-0.20%) ⬇️
sdist_mac_310_python 75.46% <58.77%> (-0.02%) ⬇️
sdist_mac_311_python 75.36% <58.77%> (-0.09%) ⬇️
sdist_mac_312_python 75.31% <58.77%> (-0.13%) ⬇️
sdist_mac_39_python 75.35% <58.40%> (-0.08%) ⬇️
sdist_win_310_python 75.47% <58.77%> (-0.07%) ⬇️
sdist_win_311_python 75.55% <58.77%> (-0.02%) ⬇️
sdist_win_312_python 75.46% <58.77%> (-0.08%) ⬇️
sdist_win_39_python 75.51% <58.40%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@paulbkoch
Copy link
Collaborator

Yes, @DerWeh, this approach looks great. Let me know when you feel it's ready to be merged.

@paulbkoch paulbkoch force-pushed the develop branch 6 times, most recently from f8501a4 to e0369a7 Compare December 10, 2024 06:25
@paulbkoch paulbkoch force-pushed the develop branch 5 times, most recently from 50d4254 to 5e999b6 Compare December 26, 2024 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

merge_ebm produces broken classifiers
2 participants