Anthony Blaom, PhD
Anthony Blaom, PhD
@tiemvanderdeure Thanks for this PR, which looks like a valuable contribution. I'll try to investigate the fail soon and get back you.
@tiemvanderdeure The source of your fail [is sorted](#250). Do you mind rebasing onto dev?
@tiemvanderdeure I'm tweaking the tests a bit at #251. I suggest you wait till I finish that before writing your tests. Sorry for any inconvenience.
Regarding tests: I don't think it is nessary to run `testoptimiser` or to compare predictions b/w CPU/GPU, as we do for the other classifier. The `basictest` should suffice.
Re documentation. In view of #252 just merged, you just need to update the table at https://github.com/FluxML/MLJFlux.jl/blob/dev/docs/src/interface/Summary.md and write a model doc-string, modelled on the existing classifier.
> Should I still wait for https://github.com/FluxML/MLJFlux.jl/pull/251 before I can update the tests? Yes, I think that's close to finished. Just waiting on the follow up review.
@tiemvanderdeure FYI #251 is now merged. Let me know if you need guidance on merge conflicts.
Okay, @tiemvanderdeure I'm keen to move this along, now that a bunch of other PR's are also coming in. I'll resolve the conflicts myself, add tests and merge myself. Will...
closed in favour of #257
> All right, thanks for finishing this @ablaom! Be sure to let me know if you need me to review or anything. Great. Definitely add you to my list :heart: