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

Bump ndarray to 0.15.0 #156

Merged
merged 9 commits into from
Aug 1, 2021
Merged

Bump ndarray to 0.15.0 #156

merged 9 commits into from
Aug 1, 2021

Conversation

YuhanLiin
Copy link
Collaborator

Addresses #124.
Having issues with new version of sprs. The compile error most likely has something to do with sparsemat/sprs#278 and was introduced by sparsemat/sprs#290.

@YuhanLiin YuhanLiin marked this pull request as draft July 31, 2021 07:19
@bytesnake
Copy link
Member

replacing the operator with sprs::binop::csmat_binop(mat.view(), mat.transpose_view(), |x, y| x.add(*y)); seems to work as a workaround. The difference between making the explicit call to add and the operator makes the difference: https://docs.rs/sprs/0.11.0/src/sprs/sparse/binop.rs.html#63

@bytesnake
Copy link
Member

bytesnake commented Jul 31, 2021

the construction of the sparse matrix fails though in the line above and I'm not yet sure why

@bytesnake
Copy link
Member

the new constructor assumes ordered indices, this solves it:

    // create CSR matrix from data, indptr and indices
    let mat = CsMatBase::new_from_unsorted((n_points, n_points), indptr, indices, data).unwrap();
    let mut mat = sprs::binop::csmat_binop(mat.view(), mat.transpose_view().to_other_storage().view(), |x, y| x.add(*y));

@mulimoen
Copy link

It seems there are some strange type inference going on when using linfa::Float in sprs. You can use the following code instead:

    // create CSR matrix from data, indptr and indices
    let mat = CsMat::<F>::new_from_unsorted((n_points, n_points), indptr, indices, data).unwrap();
    let transpose = mat.transpose_view().to_other_storage();
    sprs::binop::csmat_binop(mat.view(), transpose.view(), |_, _| F::one())

This does the same as we do behind the scene. I have assumed x + y != 0, and moved this computation into binop, which might be more performant

@YuhanLiin YuhanLiin marked this pull request as ready for review July 31, 2021 18:46
Copy link
Member

@bytesnake bytesnake left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@bytesnake bytesnake merged commit 18fa765 into rust-ml:master Aug 1, 2021
@relf relf mentioned this pull request Aug 1, 2021
11 tasks
@bytesnake
Copy link
Member

the CI pipeline somehow failed to run here as well, I checked manually ..

@YuhanLiin YuhanLiin deleted the ndarray-15 branch August 1, 2021 14:10
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