Skip to content

Conversation

@roshankern
Copy link
Member

This PR is ready for review!

In this PR, the class PR curves notebook is restructured to follow the structure of the evaluate notebooks.
This means all the models are evaluated on each data subset and the resulting data are saved in one compiled `.tsv. file.

roshankern and others added 11 commits December 9, 2022 15:51
* finish download module changes

* download notebook

* rerun split data module

* rerun download module

* rerun train_model

* rerun evaluation module

* rerun interpretation module

* combine datasets

* combine datasets

* split changes

* update format

* format update

* format

* finish split data

* combine datasets, remove holdout

* formatting

* rerun pipelines

* remove folded class

* rerun pipeline

* Update utils/download_utils.py

Co-authored-by: Dave Bunten <ekgto445@gmail.com>

* PR fixes

* module docstrings

Co-authored-by: Dave Bunten <ekgto445@gmail.com>
@roshankern roshankern requested a review from d33bs February 16, 2023 22:47
Copy link
Member

@d33bs d33bs left a comment

Choose a reason for hiding this comment

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

Nice work! I left a few suggestions but didn't see anything that would prevent a merge as-is.

One follow up I wasn't sure about:

  • I noticed your mention about using one .tsv but saw there were .png images which were also removed. Should these images be replaced by anything new?

@roshankern
Copy link
Member Author

Thanks for the review! These images should not be replaced by anything new. The format for the 3.evaluate_model module is to save the intermediate data for evaluations in .tsv and keep the image evaluations in the notebooks.

@roshankern roshankern merged commit a6aee12 into WayScience:main Feb 17, 2023
@roshankern roshankern deleted the restructure-pr-curves-notebook branch February 17, 2023 20:16
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.

2 participants