Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3D fcnn #68

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

3D fcnn #68

wants to merge 5 commits into from

Conversation

tommycwong
Copy link

@tommycwong tommycwong commented Feb 10, 2023

Updated fcnn blocks with 3D versions. I am wondering if separate 3D versions of fcnns should be created or if existing fcnns should be updated with ndim arg in fcnn.py.

Updated fcnn blocks with 3D versions. I am wondering if separate 3D versions of fcnns should be created or if existing fcnns should be updated with ndim arg in fcnn.py.
@tommycwong tommycwong changed the title 3 d fcnn 3D fcnn Feb 10, 2023
@ziatdinovmax
Copy link
Collaborator

Thank you, Tommy. I think 3D convolutions can be simply used as drop-ins. Can you please write it in a more compact way, without if/else? For example,

def get_conv(dim: int) -> Type[nn.Module]:
    conv_dict = {1: nn.Conv1d, 2: nn.Conv2d, 3: nn.Conv3d}
    return conv_dict[dim]

PS. Not sure whether List is a correct type annotation for **kwargs.

@tommycwong
Copy link
Author

I am not sure what output type I should put for the type annotation.

Copy link
Collaborator

@ziatdinovmax ziatdinovmax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tommycwong I think you need to define get_conv and get_BatchNorm (and also MaxPool?) only once, outside of all classes, and then just call those functions from each class.

@ziatdinovmax
Copy link
Collaborator

I am not sure what output type I should put for the type annotation.

nn.Module

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants