search

FluxML / MLJFlux.jl

Wrapping deep learning models from the package Flux.jl for use in the MLJ.jl toolbox
http://fluxml.ai/MLJFlux.jl/
MIT License
143 stars 17 forks source link

Bump Metalhead to 0.9 - making CUDA optional #240

Closed mohamed82008 closed 9 months ago

mohamed82008 commented 9 months ago

This PR bumps Metalhead to v0.9 which makes CUDA an optilonal dependency. Given the discussion in https://github.com/FluxML/Metalhead.jl/pull/253, I also bumped the major version of MLJFlux in the process. Please let me know if you prefer to make this a patch release and support both Metalhead 0.8 and 0.9.

mohamed82008 commented 9 months ago

It seems nightly tests fail due to a CUDA issue. It's most likely not related to this PR. Re-triggering the tests on the dev branch can confirm this.

mohamed82008 commented 9 months ago

Once this PR is approved and merged, I would appreciate a quick release.

mohamed82008 commented 9 months ago

It seems that tests are running out of time. I am trying to run them locally but they also take forever.

codecov-commenter commented 9 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (d604792) 92.08% compared to head (dbcfdc8) 92.08%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #240 +/- ## ======================================= Coverage 92.08% 92.08% ======================================= Files 12 12 Lines 316 316 ======================================= Hits 291 291 Misses 25 25 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mohamed82008 commented 9 months ago

Tests pass now.

mohamed82008 commented 9 months ago

I see that CI (at least the GH action) only has testing for latest julia version. Can we add 1.9, since 1.10 is around the corner?

The latest version which is currently tested is Julia 1.9. We also test nightly here which is more bleeding edge than 1.10. I don't understand your comment.

mohamed82008 commented 9 months ago

The CI on nightly is failing for the same reason it fails on the current dev https://github.com/FluxML/MLJFlux.jl/pull/241. I think it is a CUDA-nightly issue so should not be blocking.

ablaom commented 9 months ago

I meant add 1.9 to the matrix here, which now would mean duplicate testing. But I guess we can add that later. I'll create an issue.