Feedback from Anirban -- DLMBL 2024 - Githubissues

dlmbl / intro_dl

0 stars 4 forks source link

Feedback from Anirban -- DLMBL 2024 #4

Closed rayanirban closed 1 month ago

rayanirban commented 2 months ago

The new PyTorch ported notebooks look great! amazing job, @AlbertDominguez.

Here are few feedbacks from me:

Task 1

[x] In Cell 1, plt.rcParams["figure.figsize"] = (5, 5) -- maybe it is better to give 1 on line description of what this line does.
[x] In the plot_perceptron function, I think more explanation with comments are required for students to understand what is going on.
[x] In general, Checkpoints should also summarize what was we covered within the scope of that checkpoint. So Please add a summary.

Task 2

[x] In the description of XOR, it is better to mention XOR as a binary classification problem than x \in {0,1}^2.
[x] In the test_xor function, it is better to make xor(x) to return 1 or 0 instead of True or False.

Task 3

[x] I think it is better to give a description on why squeeze (or unsuqueeze) in y_pred = model(X_b).squeeze() is necessary in pytorch model in general.
[x] A short description of bad_model.to(device) will be helpful.
[x] A short description of y_pred = model(X_b).squeeze().detach().cpu().numpy() will be helpful.

Overall The Checkpoints should have a summary of what was covered abouve.

See you at WH 😀!

AlbertDominguez commented 1 month ago

Thanks for the feedback Anirban! I've accomodated everything you mentioned, as well as added a few more comments (and an XOR plot) to try to make stuff a bit more understandable, but nothing major.