Finished reviewing (looked at ben/denoisplit as per mattermost).
01 and 04 need some work I think but for the other ones only minor comments:
Installation
- end up in a different directory after running
source setup.sh
- recommended is putting data on our s3 bucket and download from there
- dependencies of denoiSplit not installed
01_CARE
- missing colored boxes for checkpoints, exercises etc. in the exercise notebook
- still got TODOs in there (most importantly: missing scaffolding for some tasks)
- import for
augment_batch and normalize is at the very beginning and defined by you. Maybe add a comment that makes clear that it's provided by you and add a cell with ?augment_batch and ?normalize at task 1, similarly for denormalize in task 3
- agree that calculation of means and stds should go in the function
- they already know about data loaders in principle
- the unet should come from here: https://github.com/dlmbl/dlmbl-unet
- IMO importing
torch.nn and torch.optim would be better than importing with as nn
- I got NaNs in my loss when my lr was too high
- something with the training is not working for me - validation loss is always the same
02_Noise2Void
- config explanation: I think axes being "YX" warrants more explanation (e.g. they might get 2D but be thrown for a loop by it not being "XY" )
- getting some warnings during training
- Task2: tensorboard is already started
- in export:
TypeError: CAREamist.export_to_bmz() got an unexpected keyword argument 'path' (parameter is called path_to_archive)
- Task 5: detail but data is already downloaded, so change text to "loaded in the following cell"
03_COSDD
- put explanation for dimensions before or in the task instead of after
- still has two TODOs (figure and pretrained model)
- in tensorboard metrics: 20 min in text instead of 10 min
- if you can add tensorboard logs for the pretrained models that'd be great
- Bonus
- import is wrong
- also still TODO: add pretrained
- device is not defined
04_DenoiSplit
- sys path extension isn't right
- missing dependency albumentations
- I assume this isn't done? Very light on details of what's happening
- exercise version has answers to questions
05 Noise2Noise
- text: "regression exercises" -> restoration exercises
- Task1: typo vizualize -> visualize
Finished reviewing (looked at
ben/denoisplitas per mattermost).01 and 04 need some work I think but for the other ones only minor comments:
Installation
source setup.sh01_CARE
augment_batchandnormalizeis at the very beginning and defined by you. Maybe add a comment that makes clear that it's provided by you and add a cell with?augment_batchand?normalizeat task 1, similarly fordenormalizein task 3torch.nnandtorch.optimwould be better than importing withas nn02_Noise2Void
TypeError: CAREamist.export_to_bmz() got an unexpected keyword argument 'path'(parameter is calledpath_to_archive)03_COSDD
04_DenoiSplit
05 Noise2Noise