Skip to content

Sklearn compatibility - #287

Open
darikoneil wants to merge 2 commits into
rochefort-lab:masterfrom
darikoneil:sklearn_compatibility
Open

Sklearn compatibility#287
darikoneil wants to merge 2 commits into
rochefort-lab:masterfrom
darikoneil:sklearn_compatibility

Conversation

@darikoneil

Copy link
Copy Markdown

Added decorator to adjust the arguments to the sklearn's NMF constructor such that the >=1.0 implementation's objective is identical to <= 1.0 implementation. Specifically, it generates alpha_W & alpha_H from the passed alpha parameter such that the alpha's in the objective match after scaling (there is not option to turn off scaling the alpha's in the new implementation). Results match original implementation with 1e-7 tolerance. Not sure if fix is compatible with python 2.

Added option for MiniBatchNMF with decorator so that it is also compatible.

@codecov

codecov Bot commented Jun 21, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 40.90909% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.63%. Comparing base (8cf6a17) to head (caec4d6).
⚠️ Report is 32 commits behind head on master.

Files with missing lines Patch % Lines
fissa/neuropil.py 40.90% 11 Missing and 2 partials ⚠️

❗ There is a different number of reports uploaded between BASE (8cf6a17) and HEAD (caec4d6). Click for more details.

HEAD has 149 uploads less than BASE
Flag BASE (8cf6a17) HEAD (caec4d6)
unittests 82 0
nbsmoke 67 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #287       +/-   ##
===========================================
- Coverage   94.08%   82.63%   -11.46%     
===========================================
  Files           8        8               
  Lines        1217     1215        -2     
  Branches      313      306        -7     
===========================================
- Hits         1145     1004      -141     
- Misses         36      173      +137     
- Partials       36       38        +2     
Flag Coverage Δ
nbsmoke ?
unittests ?

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@nathalierochefort

Copy link
Copy Markdown
Member

Thank you very much for the pull request, it's a great change. We won't currently integrate it directly but will point other users to this pull request if they need it, with new versions of sklearn.

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.

2 participants