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

Update argmin in linfa-linear to version 0.8.0 #289

Merged
merged 5 commits into from
Feb 10, 2023

Conversation

stefan-k
Copy link
Contributor

@stefan-k stefan-k commented Jan 29, 2023

I haven't had time to finish #236 and now rebasing is a mess. I decided to start from scratch, and this time I'll work through it crate by crate. This PR updates argmin in linfa-linear to version 0.8.0 and it makes the serde crate optional.
If this is accepted I'll open dedicated PRs for linfa-logistic and linfa-ftrl.

EDIT: Looks like argmins MSRV is now 1.62 :/

@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2023

Codecov Report

Base: 38.47% // Head: 38.50% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (3ce016c) compared to base (f816e6e).
Patch coverage: 87.50% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
+ Coverage   38.47%   38.50%   +0.03%     
==========================================
  Files          95       94       -1     
  Lines        6225     6220       -5     
==========================================
  Hits         2395     2395              
+ Misses       3830     3825       -5     
Impacted Files Coverage Δ
algorithms/linfa-linear/src/glm/hyperparams.rs 12.96% <ø> (ø)
algorithms/linfa-linear/src/glm/link.rs 18.00% <ø> (ø)
algorithms/linfa-linear/src/isotonic.rs 39.09% <ø> (ø)
algorithms/linfa-linear/src/ols.rs 59.45% <ø> (ø)
src/metrics_classification.rs 35.84% <ø> (-2.13%) ⬇️
algorithms/linfa-linear/src/glm/mod.rs 51.42% <85.71%> (+2.81%) ⬆️
src/dataset/impl_dataset.rs 43.29% <100.00%> (+0.68%) ⬆️
.../linfa-preprocessing/src/countgrams/hyperparams.rs 65.78% <0.00%> (-9.22%) ⬇️
...infa-clustering/src/appx_dbscan/cells_grid/cell.rs 41.93% <0.00%> (-4.84%) ⬇️
src/dataset/iter.rs 50.00% <0.00%> (-4.55%) ⬇️
... and 39 more

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

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

@YuhanLiin
Copy link
Collaborator

Can you check if there's any duplicate dependencies on linfa-linear now that you've bumped argmin? Also, the CI is failing because the MSRV for argmin 0.8 is higher than Linfa's MSRV. What's argmin's MSRV?

@stefan-k
Copy link
Contributor Author

stefan-k commented Feb 9, 2023

The MSRV of argmin 0.8 is 1.62. However, I can downgrade to version 0.7, which (according to lib.rs) has a MSRV of 1.59. It shouldn't make a difference for linfa-linear.

Can you check if there's any duplicate dependencies on linfa-linear now that you've bumped argmin?

Does this help? If I interpret this correctly, then there are no duplicate dependencies (apart from dev-dependencies).

❯ cargo tree --duplicates --all-features
approx v0.4.0
├── linfa v0.6.1 
│   ├── linfa-datasets v0.6.1
│   │   [dev-dependencies]
│   │   └── linfa-linear v0.6.1
│   └── linfa-linear v0.6.1
│   [dev-dependencies]
│   └── linfa-linear v0.6.1
└── ndarray v0.15.6
    ├── argmin-math v0.3.0
    │   ├── argmin v0.8.0
    │   │   └── linfa-linear v0.6.1
    │   └── linfa-linear v0.6.1
    ├── linfa v0.6.1 (*)
    ├── linfa-datasets v0.6.1 (*)
    ├── linfa-linalg v0.1.0
    │   └── linfa-linear v0.6.1 
    ├── linfa-linear v0.6.1
    ├── ndarray-csv v0.5.1
    │   └── linfa-datasets v0.6.1 (*)
    └── ndarray-linalg v0.15.0
        ├── linfa v0.6.1 (*)
        └── linfa-linear v0.6.1
[dev-dependencies]
└── linfa-linear v0.6.1

approx v0.5.1
├── nalgebra v0.29.0
│   └── statrs v0.16.0
│       [dev-dependencies]
│       └── linfa-linear v0.6.1
├── simba v0.6.0
│   └── nalgebra v0.29.0 (*)
└── statrs v0.16.0 (*)

itoa v0.4.8
├── csv v1.1.6
│   ├── linfa-datasets v0.6.1 (*)
│   └── ndarray-csv v0.5.1 (*)
└── num-format v0.4.0
    └── inferno v0.11.9
        └── pprof v0.11.0
            └── linfa v0.6.1 (*)

itoa v1.0.3
├── inferno v0.11.9 (*)
├── serde_json v1.0.83
│   ├── argmin v0.8.0 (*)
│   ├── criterion v0.4.0
│   │   ├── linfa v0.6.1 (*)
│   │   └── pprof v0.11.0 (*)
│   │   [dev-dependencies]
│   │   └── linfa-linear v0.6.1
│   ├── slog-json v2.6.1
│   │   └── argmin v0.8.0 (*)
│   └── tinytemplate v1.2.1
│       └── criterion v0.4.0 (*)
└── time v0.3.13
    └── slog-json v2.6.1 (*)

miniz_oxide v0.5.3
└── flate2 v1.0.24
    └── linfa-datasets v0.6.1 (*)

miniz_oxide v0.6.2
└── backtrace v0.3.67
    └── pprof v0.11.0 (*)

@YuhanLiin YuhanLiin merged commit 4855360 into rust-ml:master Feb 10, 2023
@stefan-k stefan-k deleted the update_argmin branch February 14, 2023 18:18
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 this pull request may close these issues.

3 participants