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

remaining issue on the mljinterface #101

Closed
2 tasks
pasq-cat opened this issue Jul 16, 2024 · 2 comments · Fixed by #103
Closed
2 tasks

remaining issue on the mljinterface #101

pasq-cat opened this issue Jul 16, 2024 · 2 comments · Fixed by #103
Assignees

Comments

@pasq-cat
Copy link
Contributor

          @Rockdeldiablo I've added the following changes now to make things work without having to overload the `update` method or changing it in MLJFlux:
  • The train method now returns la, optimiser_state, history where la is the Laplace object. This way, the object does not need to be stored as a field of the struct and the problem with update is avoided.
  • To facilitate this change, calling a Laplace object on an array, (la::AbstractLaplace)(X::AbstractArray) now simply calls the underlying neural network on data. In other words, it returns the generic predictions, not LA predictions.
  • The fitresult method was adjusted also for the classification case.

Now that tests are passing, there are a few more things to do (possibly in a new issue + PR) if you like.

  • Add a (short) tutorial to the documentation.
  • Double-check if the code in src/mlj_flux.jl can be streamlined further (e.g. do we actually still need to overload MLJFlux.build)?

For now, feel free to to focus on the other PR, just ping me and @MojiFarmanbar when you come back to this one. I need to move on to other things for now.

Originally posted by @pat-alt in #92 (comment)

@pasq-cat
Copy link
Contributor Author

pasq-cat commented Jul 16, 2024

i have been trying to cross check the various branches and i think that there is a change now that has to be implemented.

  • the new parameter ret_distr has to be added to the struct.
  • predict has to be changed both for classification and for regression

@pasq-cat pasq-cat self-assigned this Jul 18, 2024
@pasq-cat
Copy link
Contributor Author

I will work on the docs in a pull request separate from the coding part if you don't mind.

@pasq-cat pasq-cat linked a pull request Jul 19, 2024 that will close this issue
pat-alt referenced this issue Jul 22, 2024
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 a pull request may close this issue.

1 participant