Skip to content

Conversation

@ulisrael
Copy link
Contributor

I've added all the data we generated necessary for generating plots that are found in the paper

  • eval_results
  • livecell
  • metadata_LIVECELL

Added a notebook that will generate each result in the paper.

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Thanks @ulisrael - one thing immediately jumps out:

There appear to be some hard-coded paths in the notebook so it's not runnable. Could you go through the following exercise to ensure that the script is runnable and the results are reproducible:
1. Freshly clone the cellsam repo
2. Checkout this branch (git checkout --track upstream/figures)
3. Create a new virtual environment - since this is on master you'll need to use 3.10, so something like python3.10 -m venv cellsam-dev then source cellsam-dev/bin/activate.
4. Install all the dependencies pip install -r requirements.txt and cellsam: pip install .
5. Run the notebook in the way that you usually do to generate the figures

Once the figure gen is runnable, then we can move onto the next steps. Some impressions I have are:

  1. I didn't realize the results would be spread across so many files. Given that, I think it'd be best to store them outside the repo (e.g. in a bucket) to prevent tracking them.
  2. Before merging, we should also be sure that the evaluation component produces a results "package" that is runnable by this same script (e.g. the directory structure and filenames are consistent, etc.)

@rossbar
Copy link
Contributor

rossbar commented Feb 18, 2025

I took the liberty of pushing up most of the changes I mentioned above - make sure to pull them before pushing anything new here!

This still shouldn't be merged until we have a handle on the evaluation pipeline that generates these data, so IMO any spare bandwidth should go to the eval pipeline!

@ulisrael
Copy link
Contributor Author

lgtm - I slightly modified the paths so that each section can be run independently without having to run the first file.

@ulisrael ulisrael requested a review from damaggu February 24, 2025 21:12
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