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

Add test to run each notebook, reporting all errors #23

Merged
merged 4 commits into from
Apr 10, 2019

Conversation

kbattocchi
Copy link
Collaborator

No description provided.

@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch 7 times, most recently from e2d8f5e to e333cfc Compare March 27, 2019 20:56
@kbattocchi
Copy link
Collaborator Author

This set of changes does two things:

  1. Tests that we can run each jupyter notebook, collecting all errors encountered.
  2. Better modularizes our build pipeline so that the notebook and documentation tests run in parallel to our other tests (with notebooks and documentation being tested only on macOS with Python 3.6).

@kbattocchi kbattocchi added this to the v0.2 milestone Mar 27, 2019
@kbattocchi kbattocchi marked this pull request as ready for review March 27, 2019 20:59
@kbattocchi kbattocchi requested a review from moprescu March 27, 2019 21:01
@kbattocchi
Copy link
Collaborator Author

kbattocchi commented Mar 27, 2019

@moprescu How long would you expect the ortho notebook to run? It looks like that's what's causing the build failure... (timeout after 1 hr)

@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch 4 times, most recently from c515e24 to d80c995 Compare April 6, 2019 02:49
@kbattocchi
Copy link
Collaborator Author

@moprescu Even with Vasilis's perf improvements the notebooks still take 45 minutes to run. Any ideas about opportunities for further performance improvement?

@vsyrgkanis
Copy link
Collaborator

I think we should separate the OJ dataset. That’s the part that takes 45. In particular the final bootstrap. Without the final bootstrap the whole notebook just takes at most 7min. We could also just remove that final bootstrap.

@vsyrgkanis
Copy link
Collaborator

Maybe we could do a bootstrap example in the synthetic data cells above but not in the OJ

@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch from d80c995 to f663909 Compare April 8, 2019 17:58
@kbattocchi
Copy link
Collaborator Author

@vasilismsr It looks like if we just use fewer trees (say n_trees=100 instead of 2000) we can cut the time way down, at the cost of having wider confidence intervals. Then we could add some explanatory text pointing out that for greater fidelity you could use more trees but we used fewer for the sake of running more quickly. Do you see a problem with this?

@vasilismsr
Copy link
Contributor

that seems a good solution too

@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch 2 times, most recently from 0889197 to 7b3554d Compare April 9, 2019 14:52
@kbattocchi
Copy link
Collaborator Author

I've updated the notebook to use only 100 trees for the bootstrap estimation, and now all checks complete within 20 minutes.

@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch from 7b3554d to fb5e02f Compare April 10, 2019 03:06
@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch 2 times, most recently from d402f76 to 776e2c0 Compare April 10, 2019 04:17
@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch from b158e12 to fb4699e Compare April 10, 2019 13:49
@kbattocchi kbattocchi force-pushed the kebatt/testNotebooks branch from fb4699e to 23886ac Compare April 10, 2019 13:54
@kbattocchi kbattocchi merged commit 7e4c77d into master Apr 10, 2019
@kbattocchi kbattocchi deleted the kebatt/testNotebooks branch April 10, 2019 14:19
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.

4 participants