fix unet bottleneck dim off by 1 error#29
Open
PatrickRMiles wants to merge 2 commits intoLBANN:mainfrom
Open
Conversation
Collaborator
michaelmckinsey1
left a comment
There was a problem hiding this comment.
So should https://github.com/PatrickRMiles/ScaFFold/blob/506024b18f84620d970d586ca1973b83c361e5ea/ScaFFold/configs/benchmark_default.yml#L8 and https://github.com/PatrickRMiles/ScaFFold/blob/506024b18f84620d970d586ca1973b83c361e5ea/ScaFFold/configs/benchmark_testing.yml#L8 both be updated to 4 instead of 3?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adjusts how we calculate the numbers of "layers" in the unet from
problem_scaleandunet_bottleneck_dim. With this change, the bottleneck spatial dimensions are nowpow(2, unet_bottleneck_dim)as intended.Context: We noticed recently that the
unet_bottleneck_dimdid not map to the spatial dimensions of the bottleneck layers as we expected: for a dim of 3, we expected bottleneck spatial dimensions of size 8, we observed 4. This has to do with how we build the unet (or equivalently how we define "layers"). We pass a layers arg into the Unet construction. I think we have been interpreting this to mean the number of "levels" in the unet, but with how we construct the unet, it's actually "levels" - 1 (or equivalently the number of 2x2 downsampling steps we apply).We construct the unet as follows:
In ScaFFold, we calculate layers as layers = problem_scale - bottleneck_dim + 1 . At scale 6 and bottleneck dim 3, we have 6 - 3 + 1 == 4 . So in the unet, we have 1 + (layers - 1) + 1 == 1 + (4-1) + 1 == 5 "levels", four of which include 2x2 downsampling from the MaxPool3d. This means we take input of size 64, and apply 2x2 downsampling four times. 64 -> 32 -> 16 -> 8 -> 4. That's why we see bottleneck spatial dims of size 4, instead of 8. This holds regardless of problem_scale: at scale 7, we have input size 128 but five downsampling steps, so we still arrive at bottleneck spatial dim size 4.