dlmbl / boot

1 stars 3 forks source link

Reviewing Assignment #9

Closed rayanirban closed 1 month ago

rayanirban commented 1 month ago

Request to review the notebooks in the main branch inside a colab environment.

afoix commented 1 month ago

Review until Checkpoint 1

Chapter 0

Chapter 1

I will continue later/tomorrow with the review of the next Chapters. Great job @rayanirban 🥳

afoix commented 1 month ago

Review until check point 5

Chapter 2

Chapter 3

Chapter 4

Excellent job @rayanirban I am looking forward to help you with the exercise! i am sure the students will learn a lot and have a lot of fun!

edyoshikun commented 1 month ago

Here is my review of the notebook. I tested this by running the solution.py and the solution.ipynb.

Chapter 0

Chapter 1

Chapter 2

Chapter 3

Chapter 4

Chapter 5 (optional)

Great job @rayanirban and see you soon!

Ben-Salmon commented 1 month ago

Review

Hi @rayanirban, the notebook looks great. The students should get a good understanding on the foundations of handling data. The pacing of the tasks is really good too. There are just a few things I would change:

Welcome

Chapter 1

Chapter 2

Chapter 3

See you in Woods Hole!

rayanirban commented 1 month ago

@afoix: Thanks! All the changes/suggestions are incorporated.

@edyoshikun: Thanks for the feedback and suggestions. Couple of things:

For this notebook, I didn't see an environment or in-line pip installing of some packages that are

Nothing need to be installed

I often times I find myself having to add/remove dimensions. (i.e squeeze(), unsqueeze(), arr[np.newaxis,...],etc). Perhaps this can be added after Task 1.5

I have added np.newaxis. I am not adding squeeze and unsqueeze or any other thing here as we have not introduced torch tensors yet.

This one is super minor. Just highlighting that on Task 1.8 you could use tqdm in that simple for loop to demonstrate it earlier in the notebook. It's a single loop vs two loops.

I wanted students to focus on the filename printing and the loop is quite fast so I avoided adding tqdm here.

I assume in this notebook we aren't using anything fancy to look at the masks (i.e skimage.color.label2rgb is a cool one). It would be nice to show that the instance segmentation values map to some ID number. It could be as simple as adding the viridis colorbar.

Yes, we are not using anything fancy. I have written a sentence about this in Task 1.8 on how to interpret the colors in the mask and have added a note in the colorbar exercise in Chapter 5.

Task 4.1. I was testing this on the solution.py file, so I didn't have torchvision. Not sure if the google collab has it pre-installed. This refers back to my first comment in chapter 0

This is not a programming task, the point is to go over to torchvision website and familiarize with the library.

Maybe the mapping of masks to values by looking at the colorbar from Chapter 1 can go here instead. The only downside is that this is optional..

See above :)

@Ben-Salmon: Thanks for the feedback. One clarification:

The visualize function could have a titles argument to make it clearer which image is which.

As we use the visualize function in different cases, titles was not used, I think this is fine.

Once again, thank you all for your feedbacks :), can't wait to see you all in Woods Hole 👯

All issues addressed in #10

rayanirban commented 1 month ago

All issues addressed in #10