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

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

Closed
pietrotraversa opened this issue Aug 25, 2023 · 5 comments · Fixed by #459
Closed

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

pietrotraversa opened this issue Aug 25, 2023 · 5 comments · Fixed by #459

Comments

@pietrotraversa
Copy link

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 added a commit that referenced this issue Aug 25, 2023
@nwlandry
Copy link
Collaborator

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

@pietrotraversa
Copy link
Author

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
Copy link
Collaborator

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

@pietrotraversa
Copy link
Author

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
Copy link
Collaborator

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!!

nwlandry added a commit that referenced this issue Sep 1, 2023
* Fixed #458.

* Update contributors.rst

* update based on comments

* Update bigg_data.py

* Update test_bigg_data.py

* Update bigg_data.py
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 a pull request may close this issue.

2 participants