speed-up correlation selection transformers, make them deterministic - #721
Merged
Conversation
glevv
reviewed
Jan 17, 2024
Collaborator
Author
|
Observations: I could take the opportunity to speed up the selection tests for the recursive feature elimination, those are the ones failing. The documentation for the smart correlation selector, could also be improved by showcasing the other methodologies. |
solegalli
commented
Feb 18, 2024
solegalli
commented
Feb 18, 2024
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #721 +/- ##
==========================================
+ Coverage 98.16% 98.31% +0.15%
==========================================
Files 103 103
Lines 3930 3928 -2
Branches 771 764 -7
==========================================
+ Hits 3858 3862 +4
+ Misses 26 23 -3
+ Partials 46 43 -3 ☔ View full report in Codecov by Sentry. |
Morgan-Sell
pushed a commit
to Morgan-Sell/feature_engine
that referenced
this pull request
Mar 2, 2024
…eature-engine#721) * change implementation to numpy for speed * add sorted to make deterministic * improve coverage * create abstraction for finding correlated features * add attribute with dictionary * make initial edits of smart corr transformer * initial rearrangement of the smart corr functionality * refactor smart correlation selection * add test for single_feature_selection function * rename file * fix several bugs in smart correlation * expand and fix tests smart correlation * fix error version 1.4 sklearn * fix typos * minor text edits * fix pandas deprecation * fix errors in yeoohnson * fix style * update docs * update docs * fix variable type deprecation * fix all unrelated errors' * remove support for python 3.8 * comment out break with new sklearn version * add newer python versions to circleci tests' * change python images circleci * attempt to fix the correlation test error * add environments to tox ini * expand docs * add changelog * add changelog to index
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
3 participants
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.
closes #619
closes #648
closes #633
closes #612
closes #446
closes #684
closes #570
closes #703
FYI @dlaprins @glevv
Sorts variables alphabetically to avoid different results.
Adds adaptation of @dlaprins numpy implementation to speed search.
I'd like to keep the brute force correlation plain simple. And then we add the ordering per cardinality, etc to the smart correlation, which sort of has it already.
The unrelated error is in the recursive feature selectors. Something change in the estimators, or random state or something that now the values are different. Need to make that deterministic somehow.