Metalhead.jl
Metalhead.jl copied to clipboard
FluxML
Add `inchannels`, `imsize`, `nclasses` as kwargs for all constructors
I'm looking at Metalhead integration in MLJFlux. To do this well, I'm looking for some uniformity in the Metalhead.jl API that seems to be lacking. In particular, it would help if nclasses and inchannels were supported kwargs for all Metalhead constructors (VGG, and so forth). Perhaps if there are models that really can't support them, an informative error could be thrown?
For example, I think the following ought to just work but don't:
VGG(inchannels=1)
ResNet(inchannels=1)
Also helpful would be a trait to label those constructors that build fixed image size models. I think there's at least one.
nclasses is supported by every current Metalhead model, I believe. inchannels isn't but I have been working on trying to unify the API a little bit (see #174 for ResNet, for example), and so it can be easily done as part of that.
Also helpful would be a trait to label those constructors that build fixed image size models. I think there's at least one.
This might be a little complicated. Some models explicitly allow for an imsize parameter to be passed in. And while others don't, that does not necessarily mean they don't work. The Inception series is a good example - while being designed for 299x299 RGB images, they also sometimes end up working with lower resolutions. I'll see if there's a way I can ascertain this for sure
Thanks for the lightning feedback, @theabhirath.
nclasses is supported by every current Metalhead model, I believe. inchannels isn't but I have been working on trying to unify the API a little bit (see https://github.com/FluxML/Metalhead.jl/pull/174 for ResNet, for example), and so it can be easily done as part of that.
👍🏾
This might be a little complicated.
In my use-case an imsize is always available. Perhaps all models also accept imsize, but throw error/warning if that choice of imsize is definitely/possilby going to lead to unexpected behaviour.
I think we can support image size, input channels, and output classes as consistent arguments to the highest level constructors (i.e. the ones that have the pretrain keyword). I think this should be good for MLJ, since you plan to support standard architectures with tweaks on the input/output side only?
We can probably have a non-breaking release that adds the interface for this, but not all the models might fully support all the knobs. Once @theabhirath's work lands though, I think we'll be able to support it for every CNN-like architecture.
Sounds good. Be good if there is a way for me to test if a given constructor will support this extended API, however. Save me having to explicitly document which constructors are allowed for use in MLJFlux and update later.
I see two things outstanding: (1) imsize support and (2) testing whether a model allows certain options. We can easily do (1) and throw a warning when the model does not allow this kind of configuration. Is (2) required if you know a priori that every model in Metalhead has these switches?
Other issue is that we have not made a release in a long time (which is keeping these updates from reaching you). (I'll look into this part later today).
@darsnack Thanks for the update and speed response.
Is (2) required if you know a priori that every model in Metalhead has these switches?
No.
inchannels and nclasses have been added as kwargs to all the models, I believe. But imsize is not so straightforward, so support for that has not yet landed for all of the models (it is there for quite a good number of them though).
I'm not sure if this warrants another issue but the docstrings for models without imsize also don't say what the expected imsize is.
I'm not sure if this warrants another issue but the docstrings for models without
imsizealso don't say what the expected imsize is.
Oops, I'll try and work on a docs PR to solve this