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

Create one tutorial/example notebook #153

Merged
merged 36 commits into from
Feb 27, 2024
Merged

Conversation

kallewesterling
Copy link
Collaborator

I am creating a draft PR here so it's easier to have a conversation about what this looks like.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

github-actions bot commented Feb 9, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  autoemulate
  compare.py
  experimental_design.py
  plotting.py 96-97, 270
Project Total  

This report was generated by python-coverage-comment-action

add text to 01_start notebook

add text to tutorial

add text to tutorial

let save_model take a model

add tutorial updates

fix demos problem with jupyter book
@mastoffel
Copy link
Collaborator

@kallewesterling I'm still having the issue that the logs are rendered in different boxes rather than all together. I've merged you're previous PR but ch = logging.StreamHandler(sys.stdout) doesn't seem to work for me. Would you mind trying whether it looks alright for you?

@mastoffel
Copy link
Collaborator

Also just to summarise:

  1. Modified quickstart
  2. Added Tutorial
  3. Slightly changed save_model() method to take a model instead of saving the best model

@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2024

Codecov Report

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

Comparison is base (28541d5) 91.62% compared to head (89d0ca3) 91.51%.

Files Patch % Lines
autoemulate/compare.py 40.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
- Coverage   91.62%   91.51%   -0.11%     
==========================================
  Files          40       40              
  Lines        1790     1792       +2     
==========================================
  Hits         1640     1640              
- Misses        150      152       +2     

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

@kallewesterling
Copy link
Collaborator Author

@mastoffel Should we open this as a real PR?

@mastoffel
Copy link
Collaborator

@kallewesterling yes, I think so. I'm currently working on another PR which will change the tutorials slightly, so will do that first and then give the tutorials an overhaul.

add tests for new cv_run output

run both param search and cv on training data

fix print for new param search setup

fix plotting for new param search setup

let compare return best_model

add model test score

add test_score test

small fixes
rename function

cleanup

fix test
@mastoffel
Copy link
Collaborator

A few more changes, mainly:

  • worked on quickstart.ipynb
  • worked on 01_start.ipynb (more in depth tutorial)
    @kallewesterling @bryanlimy any comments on those two tutorials would be appreciated!

There are a few more changes in here (which should have probably been in another PR), but nothing big.

@kallewesterling kallewesterling marked this pull request as ready for review February 23, 2024 08:10
@kallewesterling
Copy link
Collaborator Author

Thanks @mastoffel -- I'll try to have a look at this today so we can get this PR approved before the end of the sprint!

@bryanlimy
Copy link
Member

LGTM for quickstart and 01_start.ipynb

@mastoffel
Copy link
Collaborator

@kallewesterling did you have a chance to look at the tutorials? If not, I think it should be ok to merge. They'll get a re-write anyway soon.

@kallewesterling
Copy link
Collaborator Author

I haven't had a chance, no, sorry. Quite pressed for time currently and will need to adjust the expectations in the next sprint. I'm happy with incremental dev on these things though! Feel free to merge.

@mastoffel mastoffel merged commit 69932f1 into main Feb 27, 2024
6 checks passed
@mastoffel mastoffel deleted the kallewesterling/issue125 branch August 12, 2024 14:36
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