xgi-org / xgi

CompleX Group Interactions (XGI) is a Python package for higher-order networks.
https://xgi.readthedocs.io
Other
178 stars 27 forks source link

BiGG dataset: reversed directionality of some reactions/hyperedges #458

Closed pietrotraversa closed 1 year ago

pietrotraversa commented 1 year ago

Hello, I saw that you implemented a function to read the BiGG dataset as a directed hypergraph. I have been working with this dataset for some months and I have some comments that hopefully could be useful. I believe that some reactions are not being read with the correct direction in the BiGG dataset. For example, in the e_coli_core.json model, the reaction “FORt: 1for_c -> 1for_e” is currently read by putting for_c in the head and for_e in the tail of the hyperedge (you can check the correct directionality of this reaction here or here.). This happens because xgi relies just on the stoichiometry weights when reading reactions, indeed r['metabolites']={'for_c': 1.0, 'for_e': -1.0} for FORt. Xgi should also look at the values of r["lower_bound"] and r["upper_bound"]. In the case of FORt the lower_bound= -1000 and upper_bound = 0, indicating that the real orientation of the reaction is the reversed one. Here is a code I wrote to reorder the direction of the reactions:


def reorder_reactions_df(reactions_df, verbose = True):
    reordered = [False]*len(reactions_df)
    for i in reactions_df.index:
        if reactions_df['lower_bound'].get(i)<0 and reactions_df['upper_bound'].get(i)<=0: # so the direction is <---
            mets = reactions_df['metabolites'][i].keys()
            for met in mets:
                reactions_df['metabolites'][i][met] *= -1  ## now the direction is correct
            reordered[i] = True 
            if verbose:
                print(reactions_df['id'][i], 'has been reordered')
            ## update the bound
            reactions_df.loc[i,'lower_bound'] *= -1
            reactions_df.loc[i,'upper_bound'] *= -1
    return reordered

where, I believe, reactions_df is equivalent to the xgi d_model["reactions”].

nwlandry commented 1 year ago

Hi @pietrotraversa! Thanks for raising this issue! I just made a PR that hopefully fixes this issue - let me know what you think!

pietrotraversa commented 1 year ago

It is perfect! thank you!

I was also thinking that you can use the information contained in the values r["lower_bound"] and r["upper_bound"] to understand if a reaction is reversible or not. If r["lower_bound"]<0 and r["upper_bound"]>0, then the reaction is reversible, meaning that it happens in both directions. So, for example, take a reversible reaction a <--> b, you could treat this case by assigning two hyperedges, one corresponding to a --> b and one corresponding to a <-- b. Let me know what you think!

nwlandry commented 1 year ago

@pietrotraversa I just pushed a change - let me know if this is what you were thinking before I add documentation!

pietrotraversa commented 1 year ago

Hi @nwlandry , yes! Great, thank you!

I just think that the line when you are checking if the hyperedge is empty should be with an and and not with an or. Before it was in this way right?

if not reactants and not products: 
      warn(...)
nwlandry commented 1 year ago

Good catch! I can easily change that. I did that to eliminate reactions that are non-physical, but I suppose that those cases don't occur in the dataset. Thanks!!