sparsemat / sprs

sparse linear algebra library for rust
Apache License 2.0
381 stars 45 forks source link

Add bindings to UMFPACK #327

Closed jlogan03 closed 1 year ago

jlogan03 commented 1 year ago
jlogan03 commented 1 year ago

This is ready for another CI pass whenever - format, doc, clippy, and test steps are all running locally now

jlogan03 commented 1 year ago

The functions that clippy didn't like on the last pass weren't used anymore and have been removed entirely.

jlogan03 commented 1 year ago

Looks like the static build is also going to need some work in the future and I'm just going to remove it for now. It looks extremely involved & nominal feature flag configuration doesn't appear to allow any of the static version to actually get built anyway

jlogan03 commented 1 year ago

Alrighty, ready for another pass of CI & passing lint/test locally

jlogan03 commented 1 year ago

Resolved the latest round of errors & ready for another go

jlogan03 commented 1 year ago

This is some great work! Just a couple of minor nits and this should be good to go.

I'll be happy to have a look at the static building later on. It should be possible to build this using e.g. cargo check --features suitesparse_ldl_sys/static, we can add a feature flag to more crates to make this easier to enable

Sounds good - I'll be interested to follow along and might give it another try myself eventually

jlogan03 commented 1 year ago

Good to go for another round of CI - might even run to completion this time...

jlogan03 commented 1 year ago

So, I'm not able to reproduce this CAMD test failure locally (on a mac), but my best guess is that it might be a semver-non-compliant change in cc, so I rolled the patch version for suitesparse-src's cc dep back to what it was before. Other than that, I don't think anything I've changed here should have an effect.

jlogan03 commented 1 year ago

For the first failed job, that looks like a network issue.

The second one (the failing camd tests on mac) are the same as before, and I have no idea why it's happening. I'm able to run those tests locally on mac without issue:

test result: ok. 28 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 6.39s

(py310) Jamess-MacBook-Pro:sprs_suitesparse_camd jlogan$ cargo test --features suitesparse_camd_sys/static -p sprs
jlogan03 commented 1 year ago

Ok, on further inspection, I realized that test is running on nightly - I do get that error locally running on the nightly channel, but not on stable. Seems to be a bug in nightly unrelated to the changes on this PR

mulimoen commented 1 year ago

I find the random CI error strange. I reran CI from master which gives no errors: https://github.com/sparsemat/sprs/actions/runs/4705082629

I am not sure if this is a miscompilation or a bug in some code we have here.

jlogan03 commented 1 year ago

I find the random CI error strange. I reran CI from master which gives no errors: https://github.com/sparsemat/sprs/actions/runs/4705082629

I am not sure if this is a miscompilation or a bug in some code we have here.

This is running successfully on nightly on my mac now - looks like whatever was busted on nightly, they must have fixed it overnight

jlogan03 commented 1 year ago

...aaaand it's breaking again. I guess since it's a pointer alignment issue, it will probably be intermittent

jlogan03 commented 1 year ago

...and running again - definitely intermittent

jlogan03 commented 1 year ago

The benign changes to suitesparse-src that were leftover from reversing my attempt to do the static build have been removed just in case those are somehow the problem, and this is ready for another CI attempt

jlogan03 commented 1 year ago

Figured out how to get rid of all those trailing underscores & I believe this should be ready to merge

mulimoen commented 1 year ago

Thanks a lot for your work in this @jlogan03!