refac: Move N2V masking in lightning module #365
Conversation
|
There are missing docs in the torch N2V manipulate: https://results.pre-commit.ci/run/github/607105530/1737501613.FBC6iWDHTgWR3WhJeX4o9A |
| if self.algorithm == "n2v": | ||
| x_preprocessed, *aux = self.preprocess(x) | ||
| else: | ||
| x_preprocessed = x |
There was a problem hiding this comment.
In self.preprocess, non-N2V algorithm returns Identity. Why is it not used here?
There was a problem hiding this comment.
I agree, then can remove the "if self.algorithm == "n2v""
There was a problem hiding this comment.
Because in other cases input to the Identity can be a list, which won't work. This whole construction is quite ugly, and definitely needs revisiting.
There was a problem hiding this comment.
I've just tested it and I get
>>> torch.nn.Identity()([torch.tensor([1,2,3])])
[tensor([1, 2, 3])]
This is the case where targets are a list?
I've also noticed this function is typed incorrectly batch should have the type list[torch.Tensor] I believe.
| if self.algorithm == "n2v": | ||
| x_preprocessed, *aux = self.preprocess(x) | ||
| else: | ||
| x_preprocessed = x |
There was a problem hiding this comment.
I agree, then can remove the "if self.algorithm == "n2v""
|
@melisande-c @jdeschamps @veegalinova Thanks for the comments, I'll take a look. Another thing is there's probably some inconsistencies in the configuration factories. Like I'm not sure N2V transform should be defined there |
jdeschamps
changed the title
jdeschamps
changed the title
|
A summary of the latest changes since the reviews:
There are some stuff that will need refactoring or splitting into multiple functions, but better open issues and deal with them later at this point. |
Changes Made
Additional Notes and Examples
Please ensure your PR meets the following requirements: