Update argmin to version 0.6.0

[stefan-k]

This PR updates argmin to the most recent version 0.6.0. In this version the serde dependency is optional, which addresses #48. It only concerns linfa-linear and linfa-logistic.

linfa-linear

This one was fairly easy to do. I'm unsure what the exact reasons for introducing ArgminParam were. I managed to remove ArgminParam by adding the necessary trait bounds to TweedyRegressorValidParams which avoids having to re-implement the math traits. Let me know if I missed something and I'll introduce ArgminParam again.

linfa-logistic

Here I did not remove ArgminParam here, but I think it should also be possible.

EDIT: I see now why you haven't upgraded to 0.5 ;). I guess it only makes sense to upgrade once the MSRV is increased. Also, I noticed that you want to avoid pulling in ndarray-linalg whenever possible. I somehow by accident removed the linalg feature from argmin-math and I'll have to see how to add that again (in the current setup it will always have ndarray-linalg as a dependency). I'm unsure how to proceed here. You can close this if you want but you can also leave this around for later ;)

[YuhanLiin]

MSRV will need to be bumped to 1.59 I think. Try upgrading to that version in the CI to see if it passes. I'm personally fine with this bump, since that version is already more than a year old. @bytesnake what do you think about bumping MSRV to 1.59?

I want to avoid pulling ndarray-linalg because it requires linking with external BLAS linalg libraries, which I want to place behind the blas feature. By default we use a pure-Rust linalg library instead. Thus, the dependency tree for a build with just default features should not include ndarray-linalg at all (this is enforced by the CI).

Can you also remove ArgminParams from linfa-logistic? You can probably remove the whole argmin_params.rs module. Also, I'd appreciate it if you can apply the same upgrade to linfa-ftrl, which also has argmin as a dependency. It doesn't need to be in this PR though.

[stefan-k]

@YuhanLiin thanks for your reviews! However, I wasn't done yet with the first review ;) I'll request another one when I'm ready.

Like I said in the edit of my initial post, I accidentially removed the possibility to turn off the ndarray-linalg dependency. Your suggestion to use ndarray_0_15 instead unfortunately won't compile, because there are no feature gates on ndarray-linalg in the code. I could add those (which would not be a breaking change), but I'd rather expose the option to use or not use ndarray-linalg in the "high level" features. I'll have to think about how to solve this. I'll properly add the feature gates on ndarray-lang, bump the minor version and then add the higher level features in the next major version.

Can you also remove ArgminParams from linfa-logistic? You can probably remove the whole argmin_params.rs module. Also, I'd appreciate it if you can apply the same upgrade to linfa-ftrl, which also has argmin as a dependency. It doesn't need to be in this PR though.

Sure, I'll do that. My time is a bit limited at the moment, therefore it may take a while until I'll get to it.

[stefan-k]

I managed to fix the issue, but I'm not unsure if the fix is a breaking change. To be on the safe side I'll probably release 0.7 soon, hopefully in the course of next week and then I'll continue working on this PR. Sorry about the delay.

[codecov-commenter]

Codecov Report

Base: 55.55% // Head: 55.19% // Decreases project coverage by -0.35% :warning:

Coverage data is based on head (2ad86d9) compared to base (e206bf4). Patch coverage: 57.14% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage   55.55%   55.19%   -0.36%     
==========================================
  Files          96       95       -1     
  Lines        8950     8903      -47     
==========================================
- Hits         4972     4914      -58     
- Misses       3978     3989      +11     

Impacted Files	Coverage Δ
algorithms/linfa-linear/src/glm/hyperparams.rs	12.96% <ø> (+1.85%)	:arrow_up:
algorithms/linfa-linear/src/glm/link.rs	58.00% <ø> (ø)
algorithms/linfa-linear/src/isotonic.rs	60.47% <ø> (ø)
algorithms/linfa-linear/src/ols.rs	80.76% <ø> (+1.28%)	:arrow_up:
algorithms/linfa-logistic/src/argmin_param.rs	16.66% <0.00%> (-33.34%)	:arrow_down:
algorithms/linfa-logistic/src/float.rs	0.00% <ø> (ø)
algorithms/linfa-logistic/src/lib.rs	62.20% <45.83%> (-7.00%)	:arrow_down:
algorithms/linfa-logistic/src/hyperparams.rs	28.57% <66.66%> (-6.73%)	:arrow_down:
algorithms/linfa-linear/src/glm/mod.rs	62.02% <100.00%> (+5.77%)	:arrow_up:
... and 18 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[stefan-k]

Closing in favor of #289 and other future PRs.