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

v0.0.3: merge staging into main #361

Merged
merged 205 commits into from
Nov 17, 2020
Merged

v0.0.3: merge staging into main #361

merged 205 commits into from
Nov 17, 2020

Conversation

levinwil
Copy link
Collaborator

v0.0.3

Added Features

  1. Added n_estimators to LifelongClassificationForests add_{task, transformer} functions. This allows each task to have a different number of trees.
  2. Previously, the classes in forest.py used finite_sample_correction with the only possible value of kappa being 1. We added the ability to set any kappa.

Bug Fixes

  1. Previously, the default values of all model parameters to LifelongClassification{Forest, Network} add_{task, transformer} functions were None. The value of None indicated to use the default model parameter specified in the instantiation of the LifelongClassification{Forest, Network}. But, this meant that a user would be unable to train a new tree transformer to purity (by setting max_depth = None) unless default_max_depth was None. This is an undesirable restriction. So, we changed the default value of all model parameters to LifelongClassification{Forest, Network} add_{task, transformer} functions to the string "default" - this indicates to use the default model parameter specified in the instantiation of the LifelongClassification{Forest, Network}.
  2. Updated selection of {transformer, voter, decider} data to be without replacement.
  3. Previously, we were encountering issues when the voters across bags would output posteriors of different lengths (caused by only training on a subset of the classes). We manually fixed these to ensure that all voters across bags output the same number of classes, by appending 0's to missing classes in the voter posterior estimates.
  4. Type checked all inputs X, y to {network, forest}.py classes.

Documentation Fixes

  1. Previously, many of the docs did not render correctly in Sphinx because they were formatted incorrectly. We fixed this formatting error and now all documentation renders correctly in Sphinx (and thus the web docs).

Presentation Changes

  1. transformer_voter_decider_split was changed to network_construction_proportion in network.py
  2. Minimized documentation and Python Package to only expose well-documented, tested functions that are described in the paper

@PSSF23 PSSF23 marked this pull request as draft November 13, 2020 23:26
@PSSF23
Copy link
Member

PSSF23 commented Nov 13, 2020

TODO: fix UncertaintyForest and optimize figure 2 tutorial #365

@levinwil
Copy link
Collaborator Author

levinwil commented Nov 17, 2020

@PSSF23 What do you think about merging this into main now that UF has been fixed (assuming tests pass)?

@PSSF23
Copy link
Member

PSSF23 commented Nov 17, 2020

@levinwil Yeah, it should be ready now as progress towards #1.

@PSSF23 PSSF23 marked this pull request as ready for review November 17, 2020 01:44
@levinwil
Copy link
Collaborator Author

@PSSF23 I agree it should be merged.

But I don't understand your second comment. How is this progress towards #1 ?

@PSSF23
Copy link
Member

PSSF23 commented Nov 17, 2020

@levinwil the package is becoming more sklearn compliant I believe? So each version would be progress towards #1?

Or is the issue complete already? We do have proglearn on pip.

@levinwil
Copy link
Collaborator Author

@PSSF23 I think there are many goals. One is becoming more sklearn compliant. I could be wrong but I don't think anything in this PR makes it more sklearn compliant - but that is beside the point. I understand the comment now.

As for your question, it's not sklearn compliant yet. All of the classes that inherit from an sklearn base class need to be be compliant as an sklearn estimator - it needs to pass the check_estimator function from sklearn. A more comprehensive tutorial can be found at https://scikit-learn.org/stable/developers/develop.html. The TLDR is that it's not there yet - we haven't completed #1 .

@jdey4
Copy link
Collaborator

jdey4 commented Nov 17, 2020

@levinwil Unfortunately I don't have write access. @jdey4 please accept #346 and #362 before merging this one, thank you!

Is it solved? Do you guys need me for anything?

@PSSF23
Copy link
Member

PSSF23 commented Nov 17, 2020

@jdey4 the above PRs are merged already. Please check this one for any potential issue. Thanks!

@levinwil levinwil merged commit 671ff6a into main Nov 17, 2020
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.

8 participants