Skip to content

Incorporate feedback for the AI@2025#12

Merged
edyoshikun merged 56 commits into
mainfrom
fix_for_2025
Aug 23, 2025
Merged

Incorporate feedback for the AI@2025#12
edyoshikun merged 56 commits into
mainfrom
fix_for_2025

Conversation

@edyoshikun

@edyoshikun edyoshikun commented Jul 26, 2025

Copy link
Copy Markdown
Contributor

This PR addresses #10 with following :

  • Remove Part 2 ( GAN approach for virtual staining). Note: we had already removed this in Adding the backup for part2  #9 .
  • Update the README.md to remove the Part 1 and Part2 since we are consolidating this exercise.
  • Test and update the dependencies
  • Mirror the demo we have in the VCP here.
  • Opened a paired PR in VisCy 128.
  • Address Ziwen, Dian, and Albert's review

@edyoshikun
edyoshikun marked this pull request as ready for review July 28, 2025 20:58

@AlbertDominguez AlbertDominguez left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @edyoshikun,

It's looking great, and seems to mostly run on the VMs! The main issue I found is that the cell computing SSIM/Dice seems to break for me, due to an unexpected dim given to cellpose. I've suggested the change.

I also had to play around a bit with the paths to the downloaded data. Are they all gonna download the data, or they'll use it from the shared mount?

Here's my other minor feedback/suggestions:

  • In the log_batch_jupyter, it'd be nice to add cmap=gray to display the images for coherency with the tensorboard viz.
  • In the alert containing several code snippets, It'd be nice to add the line to load phase2fluor model user trains in case there's a crash, laptop sleeps, nb disconnection, etc. (I made the suggestion, feel free to accept/ignore!)

Lmk if anything is unclear 🙂

Comment thread solution.py
Comment thread solution.py Outdated
Comment thread solution.py Outdated
Comment thread setup_student.sh

@adjavon adjavon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems good to me aside from a few minor comments! There are some types/organization things that I've been adding as I go along. I'll push those once I've merged them with your other changes :)

Comment thread solution.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple of quick notes:

  • The import cell took surprisingly long to run (~45s)
  • To use iohub in the terminal (in VSCode) they will also have to set the environment there. We could have them select the interpreter (in VSCode preferences)?
  • In "Visualize segmentation comparison: Fluorescence vs Virtual Staining vs Pretrained" it is getting: "Resizing is depricated in v4.0.1+" a few times --> this seems to be a cellpose issue?

Comment thread solution.py Outdated
Comment thread solution.py Outdated
Comment thread solution.py Outdated
Comment thread solution.py
Comment thread solution.py Outdated
Comment thread solution.py Outdated
Comment thread solution.py
Comment thread solution.py
Comment thread solution.py Outdated
@edyoshikun
edyoshikun merged commit 34b8b60 into main Aug 23, 2025
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