cobrapy icon indicating copy to clipboard operation
cobrapy copied to clipboard

[Feature] Nullspace calculation similar to COBRA (MATLAB)

Open dagl1 opened this issue 1 year ago • 11 comments

Checklist

Problem

Using the same model, implementation of add_loops (addLoopLawConstraints in cobra toolbox) takes significantly longer (30-300 seconds versus <1 second) in cobrapy than its matlab counterpart. Timing each step I find that it is the function nullspace, and specifically inside this function the line: _, s, vh = np.linalg.svd(A) that is taking up the majority if the ocmputation time (98%+).

In the matlab implementation a custom (it seems?) function for the nullspace is utilized, and I wondered if it is possible to implement and add this to how cobrapy handles the nullspace approximation? https://github.com/opencobra/cobratoolbox/blob/572cad82f50b93d7ddeab5d2b5e5e9fca9da4250/external/base/utilities/sparseNull.m#L4

Solution

Implement matlab's version of approximating the nullspace to achieve similar performance as in MATLAB

Alternatives

I attemtped using scipy's sparse svds function, but this was incompatible (produced no null space at all), as well as scipy's regular svd function (which in the few tests i did is about 2x as slow).

Anything else?

No response

dagl1 avatar May 29 '24 17:05 dagl1

This seems important to fix before v20 because vtop is affected. I'll check if I can repro with small numbers like 3 and 5.

deepthi avatar May 31 '24 19:05 deepthi

There's something special about 103 and 107 :) Works fine with 3,5,7,9,55,101,105,109,111. Given that, I'd say that this isn't critical to fix, though certainly nice to fix.

% for i in 3 5 7 9 101 105 109 111
for> do
for> vtctldclient GenerateShardRanges $i | tail -n 2 | head -n 1
for> done
  "aa-"
  "cc-"
  "db-"
  "e3-"
  "fd-"
  "fd-"
  "fd-"
  "fd-"

deepthi avatar May 31 '24 20:05 deepthi

There's something special about 103 and 107 :) Works fine with 3,5,7,9,55,101,105,109,111. Given that, I'd say that this isn't critical to fix, though certainly nice to fix.

% for i in 3 5 7 9 101 105 109 111
for> do
for> vtctldclient GenerateShardRanges $i | tail -n 2 | head -n 1
for> done
  "aa-"
  "cc-"
  "db-"
  "e3-"
  "fd-"
  "fd-"
  "fd-"
  "fd-"

Yes, its just 103 & 107 that are effected. I ran tests upto 128 shards.

bluecrabs007 avatar May 31 '24 20:05 bluecrabs007