Implement SPORF #374

parthgvora · 2020-11-20T15:44:32Z

Reference issue

Closes #351, #352.

Type of change

Feature request

What does this implement/fix?

Scikit-learn compliant implementation of SPORF written in pure Python.
- SPORF decision tree found in oblique_tree.py. Implements fit and predict functions as per sklearn requirements.
- ObliqueTreeClassificationTransformer added in transformers.py
- Unit tests for oblique_tree.py in tests/oblique_tree_test.py
Tutorial notebook for the proglearn implementation of SPORF is provided here.
- Notebook shows that proglearn SPORF produces the same results as the paper on the following UCI datasets: Hill-Valley, acute inflammation task 1, acute inflammation task 2

Additional information

Feature request for scikit-learn Oblique Random Forests written here.
Code is written with modularity in mind, to allow for easy implementation of other oblique trees. For example, implementing MORF just requires implementing a new splitter class.
Joint work of @parthgvora and @jmandavilli, with tasks split up according to provided issues.
@jdey4 @levinwil, please let us know if you would like to make any changes.

levinwil

For oblique tree, could you please make sure all of the docstrings have underscores that match the lengths of the headers. For example, line 16 should be enough underscores to cover the "Parameters" word. Also, you shouldn't have colons after those words.

This is to make sure things render in Sphinx correctly. Once you make those changes, can you please check the web docs to make sure things render appropriately? For reference, please check the other docs to see what it should look like and update until your docs looks like that.

levinwil

Also, the travis tests failed. One of the reasons is you didn't include the oblique tree in the ProgLearn package. To do so, please add it to https://github.com/parthgvora/ProgLearn/blob/staging/proglearn/__init__.py

Please update the code until the travis tests pass. If the code does not pass a test, Travis will show you those errors if you check the Travis console.

parthgvora · 2020-11-20T18:50:31Z

Thanks for the help Will! We'll make the changes and get back to you.

jdey4 · 2020-11-20T19:07:26Z

@parthgvora we also need all the benchmarks that show your implementation works as well as the main paper.

parthgvora · 2020-11-20T19:12:30Z

@parthgvora we also need all the benchmarks that show your implementation works as well as the main paper.

This should be in the notebook - sorry for any confusion. We ran the code on 3 UCI datasets that SPORF was able to classify perfectly; our code was also able to classify them perfectly. Notably, we could classify the Hill-Valley dataset perfectly, which RF was unable to do.

jdey4 · 2020-11-20T19:13:49Z

I will go through it in detail. It may take time.

parthgvora · 2020-11-20T19:14:28Z

I will go through it in detail. It may take time.

No problem - please let us know what you think :)

If you have any questions please let me know

PSSF23

@parthgvora @jmandavilli please black format your files to pass the travis test.

PSSF23 · 2020-11-21T23:22:05Z

@parthgvora there were some strange indentations of docstrings, and I fixed them for you. This fix should not affect black formatting.

levinwil · 2020-11-23T02:54:37Z

@PSSF23 It looks good to me now. What do you think?

PSSF23 · 2020-11-23T03:10:42Z

@levinwil Jayanta said he was going to look through the benchmarks. Otherwise, to my limit of knowledge, everything looks good.

levinwil · 2020-11-23T20:13:56Z

@jdey4 Have you had a chance to look this over?

jdey4 · 2020-11-24T01:39:08Z

@levinwil nope.

…o staging

levinwil · 2020-12-11T17:39:00Z

@parthgvora Please remember to add all parameters to the docstrings

parthgvora · 2020-12-11T17:42:59Z

@parthgvora Please remember to add all parameters to the docstrings

Yep, sorry! We're just going through the changes you suggested.
We'll have docstrings and the tutorial notebooks in soon, like tonight or tomorrow.

Thanks for your help!

jmandavilli · 2020-12-12T00:52:43Z

I just added the parameters to the docstrings as you suggested Will, so that should be good now. Sorry for the delay.

levinwil · 2020-12-12T03:49:43Z

This looks good to me.

@jdey4 you mentioned that you wanted to go through this - have you had the chance to?

@PSSF23 do you have any comments and/or concerns? I don’t see any glaring problems.

PSSF23 · 2020-12-12T05:08:29Z

@levinwil should the tutorials be included in the rst file?

levinwil · 2020-12-12T15:40:33Z

@PSSF23 yup!

PSSF23 · 2020-12-12T15:55:52Z

@levinwil then do you think the tutorial names should be more consistent? Like sporf_datasets & sporf_decision_boundaries or oblique_datasets & oblique_decision_boundaries?

Co-Authored-By: jmandav1 <39231283+jmandavilli@users.noreply.github.com> Co-Authored-By: parthgvora <parthgvora@gmail.com>

PSSF23 · 2020-12-12T23:17:33Z

@parthgvora @jmandavilli I added your tutorials to the rst file and resolved conflicts due to outdated branches. Yet as I said in the previous comment, I think more consistent names would help.

jmandavilli · 2020-12-13T01:54:26Z

Thank you for taking care of that @PSSF23! We can change the file names once Will confirms. Thank you guys for all of the help.

levinwil · 2020-12-13T03:21:40Z

I agree with @PSSF23: the tutorial names should be consistent. As for if you want the prefix to be oblique or sporf - that is up to you. I don’t have a strong opinion either way.

levinwil · 2020-12-14T20:03:17Z

@jmandavilli @parthgvora Have you all renamed the notebooks?

jmandavilli · 2020-12-14T21:00:03Z

@jmandavilli @parthgvora Have you all renamed the notebooks?

Hey, I'm really sorry I have a final in a couple of hours. I will rename the notebooks right after

jmandavilli · 2020-12-15T03:31:13Z

Just renamed the notebooks, sorry for the delay. I updated tutorials.rst as well.

Let us know if there is anything else. Thanks again.

…ecision_boundaries_functions.py Co-Authored-By: jmandav1 <39231283+jmandavilli@users.noreply.github.com> Co-Authored-By: parthgvora <parthgvora@gmail.com>

Co-Authored-By: jmandav1 <39231283+jmandavilli@users.noreply.github.com> Co-Authored-By: parthgvora <parthgvora@gmail.com>

jmandavilli · 2020-12-15T03:40:48Z

Thanks for catching that Hao. My bad.

PSSF23 · 2020-12-15T03:48:26Z

All of my concerns were addressed!

parthgvora and others added 5 commits November 12, 2020 20:55

staging branch finally

660eab2

all tests pass

273f960

finished documentation

fb8cb0f

fixed some bugs and finished predict tests

c4cef8b

Fixed header, unnecessary imports

2481e91

This was linked to issues Nov 20, 2020

Implement SPORF in ProgLearn: fit function, recreate SPORF figures #351

Open

Implement SPORF in Proglearn: predict function, documentation #352

Open

PSSF23 changed the title ~~Staging~~ Implement SPORF Nov 20, 2020

PSSF23 requested review from levinwil and jdey4 November 20, 2020 17:59

levinwil requested changes Nov 20, 2020

View reviewed changes

Including oblique_tree in proglearn

cf1b017

fixing importing issues

83c9f13

PSSF23 requested changes Nov 21, 2020

View reviewed changes

black formatting

a7c31f0

parthgvora requested a review from PSSF23 November 21, 2020 21:25

PSSF23 approved these changes Nov 21, 2020

View reviewed changes

Fix docstring indents

0e50784

levinwil approved these changes Nov 23, 2020

View reviewed changes

parthgvora added 4 commits December 11, 2020 11:53

black formatting

1328b75

Merge branch 'staging' of https://github.com/parthgvora/ProgLearn int…

7f876fe

…o staging

oblique trees added

ecb7b07

consistency of default params

39cf94a

made some documentation changes and created decision boundaries notebook

df391b6

PSSF23 and others added 2 commits December 12, 2020 15:12

Update tutorials.rst

8224304

Co-Authored-By: jmandav1 <39231283+jmandavilli@users.noreply.github.com> Co-Authored-By: parthgvora <parthgvora@gmail.com>

Merge branch 'staging' into staging

395dc9e

renamed notebooks

cf93af4

PSSF23 and others added 2 commits December 14, 2020 19:38

Update and rename oblique_decision_boundaries_functions.py to sporf_d…

706ef42

…ecision_boundaries_functions.py Co-Authored-By: jmandav1 <39231283+jmandavilli@users.noreply.github.com> Co-Authored-By: parthgvora <parthgvora@gmail.com>

Rename function for consistency

6e1205e

Co-Authored-By: jmandav1 <39231283+jmandavilli@users.noreply.github.com> Co-Authored-By: parthgvora <parthgvora@gmail.com>

levinwil merged commit df734df into neurodata:staging Dec 15, 2020

This was referenced Dec 15, 2020

Implement SPORF in Proglearn: predict function, documentation #352

Open

Implement SPORF in ProgLearn: fit function, recreate SPORF figures #351

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement SPORF #374

Implement SPORF #374

parthgvora commented Nov 20, 2020 •

edited

levinwil left a comment

levinwil left a comment

parthgvora commented Nov 20, 2020

jdey4 commented Nov 20, 2020

parthgvora commented Nov 20, 2020 •

edited

jdey4 commented Nov 20, 2020

parthgvora commented Nov 20, 2020 •

edited

PSSF23 left a comment

PSSF23 commented Nov 21, 2020

levinwil commented Nov 23, 2020

PSSF23 commented Nov 23, 2020

levinwil commented Nov 23, 2020

jdey4 commented Nov 24, 2020

levinwil commented Dec 11, 2020

parthgvora commented Dec 11, 2020

jmandavilli commented Dec 12, 2020

levinwil commented Dec 12, 2020

PSSF23 commented Dec 12, 2020

levinwil commented Dec 12, 2020

PSSF23 commented Dec 12, 2020 •

edited

PSSF23 commented Dec 12, 2020

jmandavilli commented Dec 13, 2020

levinwil commented Dec 13, 2020

levinwil commented Dec 14, 2020

jmandavilli commented Dec 14, 2020

jmandavilli commented Dec 15, 2020 •

edited

jmandavilli commented Dec 15, 2020

PSSF23 commented Dec 15, 2020

Implement SPORF #374

Implement SPORF #374

Conversation

parthgvora commented Nov 20, 2020 • edited

Reference issue

Type of change

What does this implement/fix?

Additional information

levinwil left a comment

Choose a reason for hiding this comment

levinwil left a comment

Choose a reason for hiding this comment

parthgvora commented Nov 20, 2020

jdey4 commented Nov 20, 2020

parthgvora commented Nov 20, 2020 • edited

jdey4 commented Nov 20, 2020

parthgvora commented Nov 20, 2020 • edited

PSSF23 left a comment

Choose a reason for hiding this comment

PSSF23 commented Nov 21, 2020

levinwil commented Nov 23, 2020

PSSF23 commented Nov 23, 2020

levinwil commented Nov 23, 2020

jdey4 commented Nov 24, 2020

levinwil commented Dec 11, 2020

parthgvora commented Dec 11, 2020

jmandavilli commented Dec 12, 2020

levinwil commented Dec 12, 2020

PSSF23 commented Dec 12, 2020

levinwil commented Dec 12, 2020

PSSF23 commented Dec 12, 2020 • edited

PSSF23 commented Dec 12, 2020

jmandavilli commented Dec 13, 2020

levinwil commented Dec 13, 2020

levinwil commented Dec 14, 2020

jmandavilli commented Dec 14, 2020

jmandavilli commented Dec 15, 2020 • edited

jmandavilli commented Dec 15, 2020

PSSF23 commented Dec 15, 2020

parthgvora commented Nov 20, 2020 •

edited

parthgvora commented Nov 20, 2020 •

edited

parthgvora commented Nov 20, 2020 •

edited

PSSF23 commented Dec 12, 2020 •

edited

jmandavilli commented Dec 15, 2020 •

edited