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

RFC: What should be the scope of the Layers module? #267

Open
theabhirath opened this issue Jan 1, 2024 · 3 comments
Open

RFC: What should be the scope of the Layers module? #267

theabhirath opened this issue Jan 1, 2024 · 3 comments
Labels
breaking discussion layers Related to the Layers module - generic layers for reuse by models

Comments

@theabhirath
Copy link
Member

This came up when I was trying to sort out the documentation. Right now, we have a Layers module which provides a lot of common functionality, like re-usable blocks and some utilities. But I realised that there are some inconsistencies:

  • Currently, while the inverted residual block functions live inside of the Layers module, the residual block functions are in plain old Metalhead.
    • Given that we export ViT and convolutional blocks from the Layers module, the residual block functions should also probably be moved here?
    • I also considered doing a drastic reduction of the module to only fundamentally re-usable layers – this would mean all the block functions would go out and leave only things like conv_bn, MHA and AdaptiveMeanMaxPool. This makes more sense from the perspective of the average user – someone who wants more functionality can just load Metalhead directly anyways.
  • The utilities don't really make sense in a Layers module (to be honest, they don't really make sense outside of the extremely specific use-cases of Metalhead) but have been documented there because of the fact that the layers needed them and they needed to go somewhere. If we go with the second suggestion in the previous point, this allows us to probably rework things to avoid documenting the utilities with the Layers, and create a separate section for developers who are more likely to need these anyways.

All of these changes in organisation would be breaking, which is why I am unsure how exactly we can implement these, and also the reason I am opening this issue to understand what the prevalent opinion is.

cc: @ToucheSir @darsnack

PS: A very Happy New Year!

@theabhirath theabhirath added discussion layers Related to the Layers module - generic layers for reuse by models breaking labels Jan 3, 2024
@darsnack
Copy link
Member

darsnack commented Jan 3, 2024

Reorganizing sounds like a good plan. We can have deprecations for anything in Layers that moves out and is unexported at the top level. If it is exported then I don't think it needs a deprecation.

@theabhirath
Copy link
Member Author

Right now, we don't export anything at the top level from Layers 😅 This is probably because at the time I felt the Layers module was always going to undergo changes. Maybe we can look at what could possibly be exported once the re-organisation is done.

@darsnack
Copy link
Member

darsnack commented Jan 3, 2024

That makes it even simpler then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking discussion layers Related to the Layers module - generic layers for reuse by models
Projects
None yet
Development

No branches or pull requests

2 participants