JuliaTrustworthyAI / LaplaceRedux.jl

Effortless Bayesian Deep Learning through Laplace Approximation for Flux.jl neural networks.
https://www.taija.org/LaplaceRedux.jl/
MIT License
39 stars 3 forks source link

Predicting using MLJ interface is slow #123

Closed pat-alt closed 4 weeks ago

pat-alt commented 4 weeks ago

@pasq-cat I think we should indeed just go with the direct MLJ interface #121 but for now I would still merge this cause it addresses the compute times.

codecov[bot] commented 4 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.48%. Comparing base (78c846e) to head (9873c55). Report is 5 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #123 +/- ## ========================================== - Coverage 96.65% 96.48% -0.17% ========================================== Files 22 22 Lines 658 655 -3 ========================================== - Hits 636 632 -4 - Misses 22 23 +1 ```

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

pat-alt commented 4 weeks ago

shoudln't the same be done in function MLJFlux.predict(model::LaplaceRegression, fitresult, Xnew)

?

Yes, I've done that

pasq-cat commented 4 weeks ago

shoudln't the same be done in function MLJFlux.predict(model::LaplaceRegression, fitresult, Xnew) ?

Yes, I've done that

btw i was sure that laplace accepted vectors as input. has anything been changed or i was wrong the whole time? the documentation says abstractarray but i distinctly remember this requirement....

pat-alt commented 4 weeks ago

shoudln't the same be done in function MLJFlux.predict(model::LaplaceRegression, fitresult, Xnew) ?

Yes, I've done that

btw i was sure that laplace accepted vectors as input. has anything been changed or i was wrong the whole time? the documentation says abstractarray but i distinctly remember this requirement....

hmm not sure where you say this https://juliatrustworthyai.github.io/LaplaceRedux.jl/stable/reference/#LaplaceRedux.predict-Tuple{LaplaceRedux.AbstractLaplace,%20AbstractArray}

pasq-cat commented 4 weeks ago

shoudln't the same be done in function MLJFlux.predict(model::LaplaceRegression, fitresult, Xnew) ?

Yes, I've done that

btw i was sure that laplace accepted vectors as input. has anything been changed or i was wrong the whole time? the documentation says abstractarray but i distinctly remember this requirement....

hmm not sure where you say this https://juliatrustworthyai.github.io/LaplaceRedux.jl/stable/reference/#LaplaceRedux.predict-Tuple{LaplaceRedux.AbstractLaplace,%20AbstractArray}

i know i know, must have been something that i have misunderstood in the first days, mah