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

Add preliminary Metalhead.jl integration #208

Merged
merged 9 commits into from
Aug 22, 2022
Merged

Add preliminary Metalhead.jl integration #208

merged 9 commits into from
Aug 22, 2022

Conversation

ablaom
Copy link
Collaborator

@ablaom ablaom commented Jun 28, 2022

edited To change syntax from image_builder(constructor)(args...; kwargs...) to image_builder(constructor, args...; kwargs...)


This PR replaces #206.

This PR fixes #162 but does so by preparing for Metalhead.jl integration (#205 ) and should be reviewed with that in mind. Full integration is not possible before FluxML/Metalhead.jl#176 is resolved.

Specifically, this PR:

Steps for completing the integration are given in comments in the new code.

Docstring

image_builder(metalhead_constructor, args...; kwargs...)

Return an MLJFlux builder object based on the Metalhead.jl constructor/type
constructor (eg, Metalhead.ResNet). Here args and kwargs are
passed to the MetalheadType constructor at "build time", along with
the extra keyword specifiers imsize=..., inchannels=... and
nclasses=..., with values inferred from the data.

Example

If in Metalhead.jl you would do

using Metalhead
model = ResNet(50, pretrain=true, inchannels=1, nclasses=10)

then in MLJFlux, it suffices to do

using MLJFlux, Metalhead
builder = image_builder(ResNet, 50, pretrain=true)

which can be used in ImageClassifier as in

clf = ImageClassifier(
    builder=builder,
    epochs=500,
    optimiser=Flux.ADAM(0.001),
    loss=Flux.crossentropy,
    batch_size=5,
)

@darsnack Any chance of a review in the next few weeks?

first attempt Metalhead integration (with hack); tests lacking

minor

add docstring comment

rm invalidated test

mv metalhead stuff out to separate src file

add show methods for Metalhead wraps

add forgotten files with tests

fix test

rename metal -> image_builder
@ablaom ablaom mentioned this pull request Jun 28, 2022
2 tasks
@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2022

Codecov Report

Merging #208 (4c8de77) into dev (14b39b1) will decrease coverage by 0.48%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##              dev     #208      +/-   ##
==========================================
- Coverage   93.22%   92.73%   -0.49%     
==========================================
  Files           9       11       +2     
  Lines         251      303      +52     
==========================================
+ Hits          234      281      +47     
- Misses         17       22       +5     
Impacted Files Coverage Δ
src/builders.jl 100.00% <ø> (ø)
src/mlj_model_interface.jl 94.20% <75.00%> (-2.58%) ⬇️
src/metalhead.jl 89.28% <89.28%> (ø)
src/core.jl 94.73% <100.00%> (ø)
src/types.jl 100.00% <100.00%> (ø)
src/utilities.jl 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@darsnack
Copy link
Member

darsnack commented Jul 9, 2022

I am not familiar with MLJFlux's code base, but as far as I can see, this looks okay. We can provide the syntax requested in the comments from the Metalhead side.

@ablaom
Copy link
Collaborator Author

ablaom commented Jul 9, 2022

We can provide the syntax requested in the comments from the Metalhead side.

That's great. Thanks for taking a look.

@ablaom
Copy link
Collaborator Author

ablaom commented Jul 14, 2022

Nightly fail looks unrelated.

@ablaom ablaom mentioned this pull request Jul 14, 2022
@ablaom ablaom closed this Aug 22, 2022
@ablaom ablaom reopened this Aug 22, 2022
@ablaom ablaom merged commit 4aae8c2 into dev Aug 22, 2022
@ablaom ablaom deleted the image-builder branch August 22, 2022 02:29
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.

Define a working default Flux chain builder for ImageClassifier
3 participants