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

some more networkapi functions ? #326

Closed
wants to merge 17 commits into from

Conversation

yewalenikhil65
Copy link
Contributor

@yewalenikhil65 yewalenikhil65 commented Apr 9, 2021

Based on #318

for now, i have written this in a form of tutorial ! Should this be in tutorial form Or should we add some more networkapi functions for incidence matrix of complexes , complex stoichiometric matrix , outgoing matrix, Laplacian matrix and so on?

updating all commits to local repo
A network dynamics approach to chemical reaction networks
updated for viewing graph for `0` or `Nothing` type of complex in reactant/product
@isaacsas
Copy link
Member

@yewalenikhil65 very nice! I think it would make more sense though to add some of these intermediate matrix representations to the API, and then have the tutorial show something cool one can do using them. What do you think?

@yewalenikhil65
Copy link
Contributor Author

yewalenikhil65 commented Apr 11, 2021

@yewalenikhil65 very nice! I think it would make more sense though to add some of these intermediate matrix representations to the API, and then have the tutorial show something cool one can do using them. What do you think?

Yes, i think having these matrices as output of some networkapi functions makes sense. I shall add these matrices into networkapi after testing more tonight.. As for tutorial to do something nice using them, i will do that in few days soon.

function `complexstoichmat(rn)` defined
export complexstoichmat
@yewalenikhil65
Copy link
Contributor Author

@isaacsas , added a complexstoichmat function to networkapi. Please check for improvements if any in the loops. I would suggest to not quickly merge, as i plan to come soon with brief tutorial of application of this function to some problem.

src/networkapi.jl Outdated Show resolved Hide resolved
@isaacsas
Copy link
Member

Could you also add tests for some examples to make sure the returned matrices are all correct?

@yewalenikhil65
Copy link
Contributor Author

Could you also add tests for some examples to make sure the returned matrices are all correct?

Yup, .. any suggestion on what kind of reactions i should preferably use for tests ?

@isaacsas
Copy link
Member

I'd suggest systems with a variety of reactions, including non-mass action reactions if they make sense, and where you know the answer (either from a paper, or from working it out by hand).

updating networkapi.jl with new API functions based on intermediate complexes in reactions
adding tests for api functions `complex_stoich_matrix` , `complex_incidence_matrix`, `complex_outgoing_matrix` and `netstoichmat` functions
exporting `reaction_complexes`, `reaction_rates`, `complex_stoich_matrix`, `complex_incidence_matrix`, `complex_outgoing_matrix`
@yewalenikhil65 yewalenikhil65 changed the title a tutorial or some more networkapi functions ? some more networkapi functions ? Apr 19, 2021
@isaacsas
Copy link
Member

@yewalenikhil65 will take a look later in the week, I’m swamped through Thursday currently.

@yewalenikhil65
Copy link
Contributor Author

@yewalenikhil65 will take a look later in the week, I’m swamped through Thursday currently.

No problem

@yewalenikhil65
Copy link
Contributor Author

Hi @isaacsas
Didn't get time to follow up this later in past month(COVID issues at home)

There example is missing a demonstration for the use of these functions. Will start to work on it this week.

updating the tutorial to accomodate changes as per symbolics array
@isaacsas
Copy link
Member

@yewalenikhil65 what is the status on this PR? Are you done with it? (Your last comment said there was more you planned to do.)

@yewalenikhil65
Copy link
Contributor Author

yewalenikhil65 commented Jul 23, 2021

@yewalenikhil65 what is the status on this PR? Are you done with it? (Your last comment said there was more you planned to do.)

hi @isaacsas wrapping it up here yewalenikhil65#2 (comment)
would you please take a look and suggest/improve it ?
should this be okay as a tutorial for demo of these new functions ?

@yewalenikhil65
Copy link
Contributor Author

@isaacsas closing this PR , and will come up with new PR with functions + proposed tutorial

@isaacsas
Copy link
Member

@yewalenikhil65 sorry for my delays with this one. If you feel this is done and in good shape I can review; I just wasn't clear if you had further updates you intended to make or not!

I'll look through your comment -- somehow I missed that in the previous discussion.

@yewalenikhil65
Copy link
Contributor Author

@yewalenikhil65 sorry for my delays with this one. If you feel this is done and in good shape I can review; I just wasn't clear if you had further updates you intended to make or not!

I'll look through your comment -- somehow I missed that in the previous discussion.

@isaacsas sure.. please do review yewalenikhil65#2 (comment) , so that we can decide whether it goes well with the complex-stoichiomatrices matrices functions in networkapi.jl that i proposed earlier.
I will add a new PR for this one.

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.

None yet

2 participants