[ENH] Add option to permute per forest fraction

[adam2392]

Discussed in Forest meeting today:

Changes proposed in this pull request:

• adds a permute_per_forest_fraction parameter that permutes the covariate_index a controlled number of times over the entire forest (rather than per tree)
• Helps resolve the humongous usage of RAM when using large X, or large forests, or many repeated jobs of FeatureImportance*Classifier.

Before submitting

• I've read and followed all steps in the Making a pull request
  section of the CONTRIBUTING docs.
• I've updated or added any relevant docstrings following the syntax described in the
  Writing docstrings section of the CONTRIBUTING docs.
• If this PR fixes a bug, I've added a test that will fail without my fix.
• If this PR adds a new feature, I've added tests that sufficiently cover my new functionality.

After submitting

• All GitHub Actions jobs for my pull request have passed.

[codecov]

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (a055049) 88.86% compared to head (f3aa7d7) 89.06%.

❗ Current head f3aa7d7 differs from pull request most recent head 9d0f2db. Consider uploading reports for the commit 9d0f2db to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #145      +/-   ##
==========================================
+ Coverage   88.86%   89.06%   +0.20%     
==========================================
  Files          41       41              
  Lines        3439     3531      +92     
==========================================
+ Hits         3056     3145      +89     
- Misses        383      386       +3     

Files	Coverage Δ
sktree/experimental/mutual_info.py	20.51% <ø> (ø)
sktree/stats/permutationforest.py	77.39% <100.00%> (+0.19%)	⬆️
sktree/stats/tests/test_coleman.py	100.00% <ø> (ø)
sktree/stats/tests/test_forestht.py	98.46% <100.00%> (-1.09%)	⬇️
sktree/stats/utils.py	91.89% <100.00%> (ø)
sktree/tree/_honest_tree.py	99.43% <ø> (ø)
sktree/tree/tests/test_all_trees.py	100.00% <100.00%> (ø)
sktree/tree/tests/test_tree.py	100.00% <100.00%> (+0.48%)	⬆️
sktree/stats/forestht.py	95.54% <91.25%> (+0.23%)	⬆️

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

[adam2392]

I'll let #143 get merged first, so I can test out changes with this new functionality

[PSSF23]

The tests didn't pass because some indices were not stratified during testing. I also feel the samples variable is used too many times, which could cause confusion. In classifier _statistic, samples could mean test indices or non-nan indices.

[PSSF23]

Should we close this?

[adam2392]

It's a simple addition that's backwards compatible so I think I can just finish adding it.

[adam2392]

Is this mergable? @PSSF23 to preserve the old behavior, just set permute_forest_fraction = 1.0 / n_estimators.

[PSSF23]

@adam2392 We should keep the same behavior for the permute_per_tree parameter, so Sam would not need to change his code. The permute fraction should default to each tree with None value, similar to how sklearn handles the relationship between max_samples and bootstrap. Like raising an error if fraction is not None when permute_per_tree is False.

[adam2392]

@adam2392 We should keep the same behavior for the permute_per_tree parameter, so Sam would not need to change his code. The permute fraction should default to each tree with None value, similar to how sklearn handles the relationship between max_samples and bootstrap. Like raising an error if fraction is not None when permute_per_tree is False.

Currently, permute_per_tree default for Sam is False corresponding to no permutation per tree.

With this PR, permute_per_forest_fraction = None would have the same default. Is this what you mean?

[PSSF23]

@adam2392 Actually it might not be relevant to Sam at the current stage of implementation as he's only running statistics.

My original thought is to preserve any current use of permute_per_tree and treat permute_per_forest_fraction as add-on to it. But as the new parameter fully covers the original use, I think it's fine to upgrade. Can you fix the error in the regressor test?