Repository navigation
Mg/update DTEx252 - #533
mirjagranfors wants to merge 6 commits into
Conversation
| "\n", | ||
| " rng_state = torch.get_rng_state()\n", | ||
| " np_state = np.random.get_state()\n", | ||
| " cuda_state = torch.cuda.get_rng_state(device)\n", |
There was a problem hiding this comment.
issue (blocking): The notebook selects CPU when CUDA is unavailable, but resolve_paired_sample() then calls torch.cuda.get_rng_state(device) unconditionally. You should guard both of the CUDA random state calls for CPU.
There was a problem hiding this comment.
Fixed: added a use_cuda flag and both torch.cuda.get_rng_state and set_rng_state calls are now only run when CUDA is used.
There was a problem hiding this comment.
note: The phase-mask and baseline sections repeat most of the training/validation loop, evaluation and plotting, with few things changed. Maybe a train_model(...) and evaluate_model(...) helper could keep the notebooks smaller and more readable. Just an idea for future notebooks, I don't think a refactor is needed here.
There was a problem hiding this comment.
Yes, I agree. I'll keep it in mind for future tutorials.
This branch has not been deployed
3 participants
Updating tutorial for the phase mask.
Since training one of the CNNs takes about 2 days on my computer, and the entire notebook therefore takes about 4 days to run, I have added the option to use the weights and phase mask from my training.
For the figure in the paper, I want the comparison between the cases with and without the phase mask to be as fair as possible. Therefore, I have made the examples for calculating the metrics and showing the results in 3D use the exact same number of particles, particle positions, and noise. However, I am not sure whether it is a good idea to add this as a separate notebook (which is what I did in this PR) or if there is a better way to do it.