BioSystemsUM / troppo

Reconstruction algorithms for Python
GNU General Public License v3.0
22 stars 5 forks source link

tINIT error with `essential_reactions` and `no_reverse_loops` inputs #13

Closed JonathanRob closed 1 year ago

JonathanRob commented 3 years ago

The tINIT model extraction method fails whenever specifying one or more essential_reactions, or if no_reverse_loops is set to True. It seems like it has something to do with the construction of the irreversible S-matrix.

For example, your INIT_test.py script returns an error when running the t.preprocessing() command:

ValueError                                Traceback (most recent call last)
<ipython-input-10-f12e9733ed08> in <module>
     27 t = tINIT(S, lb, ub, tINITProperties(reactions_scores=asd, present_metabolites=[], essential_reactions=[8],
     28  production_weight=0.0, allow_excretion=False, no_reverse_loops=True))
---> 29 t.preprocessing()
     30 #t.build_problem()
     31 #res = t.solve_problem()

.../troppo/methods/reconstruction/tINIT.py in preprocessing(self)
    182                 matrix_to_add = sprs.eye(self.n_reactions_irrev, format='csc')[:, ids_to_keep]
    183 
--> 184         self.irreversible_S = sprs.vstack(
    185             [self.irreversible_S, matrix_to_add])
    186 

.../scipy/sparse/construct.py in vstack(blocks, format, dtype)
    497 
    498     """
--> 499     return bmat([[b] for b in blocks], format=format, dtype=dtype)
    500 
    501 

.../scipy/sparse/construct.py in bmat(blocks, format, dtype)
    594                                                     exp=bcol_lengths[j],
    595                                                     got=A.shape[1]))
--> 596                     raise ValueError(msg)
    597 
    598     nnz = sum(block.nnz for block in blocks[block_mask])

ValueError: blocks[:,0] has incompatible row dimensions. Got blocks[1,0].shape[1] == 9, expected 10.

After changing the no_reverse_loops option to False, and the essential_reactions input to an empty list [], the code will run without error. Personally I'm not too worried about the no_reverse_loops option, but it would be nice to be able to specify the essential reactions to keep.

I'm using troppo version 0.0.5.

vvvieira commented 3 years ago

Hello @JonathanRob,

This bug occurred when changing from dense to sparse matrices. One of the matrix slices was being performed by column and not by row.

We still have a long way to go as far as testing/CI is concerned so we missed this one completely.

Thank you very much for reporting! This will be fixed on PyPI once version 0.0.6 is out.

JonathanRob commented 3 years ago

Fantastic, thank you @skapur!