Setting up a UNet model#46
Conversation
Co-authored-by: Maria Novitasari <mnovitasari@turing.ac.uk> Co-authored-by: Louisa van Zeeland <louisa-ai2@protonmail.com> Co-authored-by: James Robinson <james.em.robinson@gmail.com>
IFenton
changed the title
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
jemrobinson
left a comment
jemrobinson
left a comment
There was a problem hiding this comment.
This is broadly OK, but needs some tidying up. Can you take care of this @IFenton ?
Thanks for the comments - I'm working on this. I've also been thinking about adding some basic tests for the models, but maybe that should be a separate PR? What do you think @jemrobinson? |
|
Always good to add tests, but don't let perfect be the enemy of good. Let's try to merge PRs once they're good enough. |
Co-authored-by: James Robinson <james.em.robinson@gmail.com>
Co-authored-by: James Robinson <james.em.robinson@gmail.com>
Co-authored-by: James Robinson <james.em.robinson@gmail.com>
Co-authored-by: James Robinson <james.em.robinson@gmail.com>
|
|
||
| start_out_channels = 64 | ||
|
|
||
| reduced_channels = int(start_out_channels * n_filters_factor) |
There was a problem hiding this comment.
Wouldn't it be easier to make reduced_channels an argument rather than having n_filters_factor as an argument that we multiply by 64?
There was a problem hiding this comment.
I think it's actually neatest to just let people set the start_out_channels argument in the config file, then there is no need for the n_filters_factor or the reduced_channels
jemrobinson
left a comment
jemrobinson
left a comment
There was a problem hiding this comment.
A couple of minor fixes, plus some thoughts about code structure.
Co-authored-by: James Robinson <james.em.robinson@gmail.com>
| import torch.nn as nn | ||
| from torch import Tensor | ||
| import torch | ||
| import torch.nn.functional as F |
There was a problem hiding this comment.
I think we don't use this anymore
There was a problem hiding this comment.
Which 'this'? I think we do need to import those four things
There was a problem hiding this comment.
Sorry, I was trying to point to import torch.nn.functional as F
jemrobinson
force-pushed
the
43-add-unet
branch
from
032350d to
4b01b69
Compare
jemrobinson
force-pushed
the
43-add-unet
branch
from
4b01b69 to
5e8bc13
Compare
2 participants
Closes #43