add constructor for binary classifiers

[tiemvanderdeure]

This PR adds a separate constructor NeuralNetworkBinaryClassifier for binary classification.

Unlike for the existing NeuralNetworkClassifier constructor, in the final layer always has size 1. By default, a sigmoid function is used to transform this layer. This is a pretty common thing to use neural networks for, and as far as I can tell it's not possible with the existing constructors.

I'll add tests and documentation if this PR gets some support.

PR Checklist

• Tests are added
• Documentation, if applicable

[tiemvanderdeure]

I think the test failures are unrelated to this PR?

[ablaom]

@tiemvanderdeure Thanks for this PR, which looks like a valuable contribution. I'll try to investigate the fail soon and get back you.

[ablaom]

@tiemvanderdeure The source of your fail is sorted. Do you mind rebasing onto dev?

[ablaom]

Thanks @tiemvanderdeure for this contribution and taking the time to wrap your head around the code base.

I can see attention to detail in the PR, which is much appreciated. I couldn't spot any flaws in your implementation. Are you happy to add some tests?

[tiemvanderdeure]

Thanks for reviewing. I'll add some tests and documentation later this week!

[ablaom]

@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.

[ablaom]

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.

[ablaom]

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.

[tiemvanderdeure]

I just added some quick docs - most of it is copied over from the docs for NeuralNetworkClassifier.

Should I still wait for #251 before I can update the tests?

[codecov]

Codecov Report

Attention: Patch coverage is 17.64706% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 88.18%. Comparing base (ec6004f) to head (5842ed9).
Report is 1 commits behind head on dev.

Files	Patch %	Lines
src/classifier.jl	18.18%	9 Missing ⚠️
src/core.jl	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #248      +/-   ##
==========================================
- Coverage   92.08%   88.18%   -3.91%     
==========================================
  Files          12       12              
  Lines         316      330      +14     
==========================================
  Hits          291      291              
- Misses         25       39      +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ablaom]

Should I still wait for #251 before I can update the tests?

Yes, I think that's close to finished. Just waiting on the follow up review.

[ablaom]

@tiemvanderdeure FYI #251 is now merged. Let me know if you need guidance on merge conflicts.

[ablaom]

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 let you know if I encounter problems.

Thanks indeed for this valuable contribution 🙏🏾

[ablaom]

closed in favour of #257

[tiemvanderdeure]

All right, thanks for finishing this @ablaom! Be sure to let me know if you need me to review or anything.

[ablaom]

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 ❤️