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

Optimize UncertaintyForest() notebooks and add Figure 2 tutorial #365

Merged
merged 24 commits into from
Nov 17, 2020

Conversation

EYezerets
Copy link
Collaborator

Reference issue

Contributes to addressing Issue 37: make 3 tutorials to run the experiments from uncertainty-forests

Type of change

Documentation and Bug Fix

What does this implement/fix?

This creates a new tutorial to run Figure 2 from this paper with a small dataset (similar to the tutorial created by @bstraus1 for Figure 1). The functions called by the Figure 2 tutorial are located in unc_forest_tutorials_functions.py so as not to clutter the notebook.
I also implemented a small bug fix to make sure predict_proba() in UncertaintyForest() can run by setting self.lf_ in the __init__() function.

Additional information

Copy link
Member

@PSSF23 PSSF23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think any output file should be uploaded? @EYezerets

@EYezerets
Copy link
Collaborator Author

@PSSF23 Oh good point, yes I can remove the pkl files, but I will leave an empty output folder if that's all right

@PSSF23
Copy link
Member

PSSF23 commented Nov 13, 2020

@EYezerets I believe the best thing, if feasible, would be having everything contained in the notebook and leaving no output file at all. So instead of saving the outputs as pickle files and loading again, the program uses them directly to generate the figures.

That was a suggestion Will gave to me and Yuta when we simplified the benchmark tutorials (random_class_exp & label_shuffle_exp). @levinwil do you think the rule apply here as well?

@levinwil
Copy link
Collaborator

@EYezerets @PSSF23 Yes I agree with that suggestion!

@levinwil
Copy link
Collaborator

levinwil commented Nov 16, 2020

@EYezerets Any updates? This is the last PR before merging staging into master.

@EYezerets
Copy link
Collaborator Author

Hi @levinwil , just made a few edits today so it no longer outputs pkl files and still plots Figure 2 with the test parameters. Please kindly let me know if there's anything else needed before merging!

@PSSF23
Copy link
Member

PSSF23 commented Nov 16, 2020

@EYezerets Please add your tutorial name to docs/tutorials.rst and remove the last blank cell in the fig2 notebook, thanks. And in the first markdown cells of all uncertainty forest tutorials, are you actually referring to this notebook: installation_guide.ipynb?

@PSSF23
Copy link
Member

PSSF23 commented Nov 16, 2020

Actually, could you rewrite all the uncertaintyforest tutorials' introduction to have a consistent style? These three should be a set I believe?

@EYezerets
Copy link
Collaborator Author

@PSSF23 Yes I can do that! (Just to clarify, the first two tutorials were created by Ben, and once I make the one for Figure 3, there will be 4 uncertainty forest tutorials total. I will update the existing 3 tutorials to be as consistent as possible, and please let me know if you have additional feedback.)

@EYezerets
Copy link
Collaborator Author

@PSSF23 Could I please verify with you whether the way I wrote the figure 2 tutorial matches up better with the Netlify site requirements? That was my goal, and I was planning to modify the other two notebooks to be consistent with that.

@levinwil
Copy link
Collaborator

@EYezerets On the checks, go to any of the Netlify checks. Click on "details" on the right. Then navigate to your tutorial. You'll see how it renders.

@PSSF23
Copy link
Member

PSSF23 commented Nov 16, 2020

@EYezerets Basically, what is going to show up on the Netlify website would be "#" formatted title and "##" formatted subtitles. I think what you have on fig2 tutorial should be mostly good. And you must add your tutorial name to docs/tutorials.rst for it to show up at all. Check out more at #356

@PSSF23
Copy link
Member

PSSF23 commented Nov 16, 2020

I resolved the rst conflicts. Feel free to add more changes and let us know when you are done @EYezerets

@EYezerets
Copy link
Collaborator Author

Wait just a moment! I just ran the notebook so I have output for it again!

@EYezerets
Copy link
Collaborator Author

Thank you @PSSF23

@PSSF23 PSSF23 changed the title UncertaintyForest() Figure 2 tutorial Optimize UncertaintyForest() notebooks and add Figure 2 tutorial Nov 16, 2020
@PSSF23
Copy link
Member

PSSF23 commented Nov 16, 2020

@EYezerets I would prefer a version without the warnings but the website is looking good! And there is still a blank cell at the end of uncertaintyforest_running_example~

@EYezerets
Copy link
Collaborator Author

Ah hmm sorry about the warnings - it may be a side effect of running on AWS, but my computer is not unserializing parallel tasks if I run the tutorial locally.
I can take out the extra cell in that notebook right away - thanks for catching that!

@levinwil
Copy link
Collaborator

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

Copy link
Member

@PSSF23 PSSF23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@levinwil I think it's good, too! Everything I know is addressed.

@PSSF23 PSSF23 merged commit ec29e47 into neurodata:staging Nov 17, 2020
@EYezerets
Copy link
Collaborator Author

That's awesome! Thank you both! @PSSF23 @levinwil

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.

3 participants