Skip to content

Remove blas from linfa clustering #257

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Nov 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,4 +51,4 @@ jobs:
toolchain: ${{ matrix.toolchain }}

- name: Run cargo test with BLAS enabled
run: cargo test --release --workspace --features intel-mkl-static,linfa-clustering/blas,linfa-ica/blas,linfa-reduction/blas,linfa-linear/blas,linfa-preprocessing/blas,linfa-pls/blas,linfa-elasticnet/blas
run: cargo test --release --workspace --features intel-mkl-static,linfa-ica/blas,linfa-reduction/blas,linfa-linear/blas,linfa-preprocessing/blas,linfa-pls/blas,linfa-elasticnet/blas
1 change: 0 additions & 1 deletion algorithms/linfa-clustering/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ categories = ["algorithms", "mathematics", "science"]

[features]
default = []
blas = ["ndarray-linalg", "linfa/ndarray-linalg"]
serde = ["serde_crate", "ndarray/serde", "linfa-nn/serde"]

[dependencies.serde_crate]
Expand Down
3 changes: 1 addition & 2 deletions algorithms/linfa-clustering/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ Implementation choices, algorithmic details and a tutorial can be found
**WARNING:** Currently the Approximated DBSCAN implementation is slower than the normal DBSCAN implementation. Therefore DBSCAN should always be used over Approximated DBSCAN.

## BLAS/Lapack backend

See [this section](../../README.md#blaslapack-backend) to enable an external BLAS/LAPACK backend.
We found that the pure Rust implementation maintained similar performance to the BLAS/LAPACK version and have removed it with this [PR](https://github.com/rust-ml/linfa/pull/257). Thus, to reduce code complexity BLAS support has been removed for this module.

## License
Dual-licensed to be compatible with the Rust project.
Expand Down
44 changes: 1 addition & 43 deletions algorithms/linfa-clustering/src/gaussian_mixture/algorithm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,9 @@ use crate::gaussian_mixture::hyperparams::{
GmmCovarType, GmmInitMethod, GmmParams, GmmValidParams,
};
use crate::k_means::KMeans;
#[cfg(feature = "blas")]
use linfa::dataset::{WithLapack, WithoutLapack};
use linfa::{prelude::*, DatasetBase, Float};
#[cfg(not(feature = "blas"))]
use linfa_linalg::{cholesky::*, triangular::*};
use ndarray::{s, Array, Array1, Array2, Array3, ArrayBase, Axis, Data, Ix2, Ix3, Zip};
#[cfg(feature = "blas")]
use ndarray_linalg::{cholesky::*, triangular::*};
use ndarray_rand::rand::Rng;
use ndarray_rand::rand_distr::Uniform;
use ndarray_rand::RandomExt;
Expand Down Expand Up @@ -265,14 +260,6 @@ impl<F: Float> GaussianMixtureModel<F> {
let n_features = covariances.shape()[1];
let mut precisions_chol = Array::zeros((n_clusters, n_features, n_features));
for (k, covariance) in covariances.outer_iter().enumerate() {
#[cfg(feature = "blas")]
let sol = {
let decomp = covariance.with_lapack().cholesky(UPLO::Lower)?;
decomp
.solve_triangular_into(UPLO::Lower, Diag::NonUnit, Array::eye(n_features))?
.without_lapack()
};
#[cfg(not(feature = "blas"))]
let sol = {
let decomp = covariance.cholesky()?;
decomp.solve_triangular_into(Array::eye(n_features), UPLO::Lower)?
Expand Down Expand Up @@ -495,19 +482,10 @@ impl<F: Float, D: Data<Elem = F>> PredictInplace<ArrayBase<D, Ix2>, Array1<usize
mod tests {
use super::*;
use approx::{abs_diff_eq, assert_abs_diff_eq};
#[cfg(feature = "blas")]
use lax::error::Error;
use linfa_datasets::generate;
use ndarray::{array, concatenate, ArrayView1, ArrayView2, Axis};

#[cfg(not(feature = "blas"))]
use linfa_linalg::LinalgError;
#[cfg(not(feature = "blas"))]
use linfa_linalg::Result as LAResult;
#[cfg(feature = "blas")]
use ndarray_linalg::error::LinalgError;
#[cfg(feature = "blas")]
use ndarray_linalg::error::Result as LAResult;
use ndarray::{array, concatenate, ArrayView1, ArrayView2, Axis};
use ndarray_rand::rand::prelude::ThreadRng;
use ndarray_rand::rand::SeedableRng;
use ndarray_rand::rand_distr::{Distribution, StandardNormal};
Expand All @@ -531,9 +509,6 @@ mod tests {
}
impl MultivariateNormal {
pub fn new(mean: &ArrayView1<f64>, covariance: &ArrayView2<f64>) -> LAResult<Self> {
#[cfg(feature = "blas")]
let lower = covariance.cholesky(UPLO::Lower)?;
#[cfg(not(feature = "blas"))]
let lower = covariance.cholesky()?;
Ok(MultivariateNormal {
mean: mean.to_owned(),
Expand Down Expand Up @@ -625,12 +600,6 @@ mod tests {

match gmm.expect_err("should generate an error with reg_covar being nul") {
GmmError::LinalgError(e) => {
#[cfg(feature = "blas")]
assert!(matches!(
e,
LinalgError::Lapack(Error::LapackComputationalFailure { return_code: 2 })
));
#[cfg(not(feature = "blas"))]
assert!(matches!(e, LinalgError::NotPositiveDefinite));
}
e => panic!("should be a linear algebra error: {:?}", e),
Expand All @@ -654,17 +623,6 @@ mod tests {
.reg_covariance(0.)
.fit(&dataset);

#[cfg(feature = "blas")]
match gmm.expect_err("should generate an error with reg_covar being nul") {
GmmError::LinalgError(e) => {
assert!(matches!(
e,
LinalgError::Lapack(Error::LapackComputationalFailure { return_code: 1 })
));
}
e => panic!("should be a linear algebra error: {:?}", e),
}
#[cfg(not(feature = "blas"))]
gmm.expect_err("should generate an error with reg_covar being nul");

// Test it passes when default value is used
Expand Down