Skip to content

Conversation

isaacsas
Copy link
Member

@yewalenikhil65 so as not to pollute your original PR I created a branch off it and made updates. What do you think? The tests you added now all pass for me.

yewalenikhil65 and others added 9 commits July 29, 2021 12:01
Complex stoichiometric matrices, reaction rates etc functions
adding some tests for functions related to complex-based matrices of reaction network
export the functions 
reaction_complexes, reaction_rates, complex_stoich_matrix, complex_incidence_matrix, complex_outgoing_matrix
adding some more tests to see whether Z*B == netstoichmat(rn)'
revising function definitions of complex based representations based
@isaacsas
Copy link
Member Author

isaacsas commented Aug 11, 2021

It this looks good to you I'll make some more tweaks and merge if tests pass.

@yewalenikhil65
Copy link
Contributor

yewalenikhil65 commented Aug 11, 2021

@yewalenikhil65 so as not to pollute your original PR I created a branch off it and made updates. What do you think? The tests you added now all pass for me.

Hi @isaacsas ,
It looks very good. I checked the tests too. I think only exporting these functions to Catalyst.jl is remaining ? I made a little PR into this new branch of yours!
I guess we should merge..

Next PRs could involve plotting visualising these complexes on graphs and Loading reaction system based on Z, B matrices as function arguments ? I have small demos of these.

@isaacsas
Copy link
Member Author

I think creating a ReactionSystem from Z,B matrices is for ReactionNetworkImporters. We can add a new importer for that type of matrix input. Feel free to open a PR there if you want.

Adding visualization here makes sense.

@isaacsas isaacsas merged commit 3c1bebf into SciML:master Aug 11, 2021
@isaacsas isaacsas deleted the reaction_complexes_v2 branch August 11, 2021 15:27
@isaacsas
Copy link
Member Author

@yewalenikhil65 nice job on this one!

@isaacsas
Copy link
Member Author

It would also be good to add a new tutorial showing how to use these matrices to do something interesting (maybe once you add the visualization features).

@yewalenikhil65
Copy link
Contributor

@isaacsas Thanks sir for your inputs.

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 this pull request may close these issues.

2 participants