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

Generating reactions programmatically using Reaction Systems #307

Merged
merged 19 commits into from
Mar 3, 2021

Conversation

yewalenikhil65
Copy link
Contributor

@yewalenikhil65 yewalenikhil65 commented Feb 26, 2021

Tutorial for Implementation of (Smoluchowski coagulation equation) population balance equation- by programmatically generating the reactions using ReactionSystem
Based on discussion at #292

This is a brief tutorial for generating reactions programmatically. Smoluchowski coagulation equations(population balance approach) system was found as a suitable example here as demonstration and we solve the system using Jump processes
example plot result for generating_reactions_programmatically.md
@isaacsas
Copy link
Member

@yewalenikhil65 I'll go through tomorrow morning and give you some comments. Got busy with meetings and class stuff today.

@yewalenikhil65
Copy link
Contributor Author

walenikhil65 I'll go through tomorrow morning and give you some comments. Got busy with meetings and class stuff today.

No problem ,Sir

@isaacsas
Copy link
Member

isaacsas commented Mar 2, 2021

@yewalenikhil65 I started making some edits, but then ran into a couple questions. See below.

@isaacsas
Copy link
Member

isaacsas commented Mar 2, 2021

You should be able to git pull to sync my changes back to your local git repository.

@yewalenikhil65
Copy link
Contributor Author

Hey @isaacsas updated according to your suggestions..

@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

@yewalenikhil65 made some more updates. Slowly getting through it. (This week is pretty busy.)

I do have one question for you, see below.

@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

Actually never mind; I forgot that I had figured out what I was confused about!

I Made a mistake in writing constant kernel case
. `kv = fill(C/V, nr)` should work. Earlier it was simply a single scalar..
@yewalenikhil65
Copy link
Contributor Author

Corrected a little mistake I spotted in writing kv for constant kernel case.. I like your changes in wording too

@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

OK, I updated it one more time. I'd be happy to merge it now. Some notes:

  • You weren't actually using ModelingToolkit / Catalyst in your previous code. When you created the MassActionJump and passed it to JumpProblem you were actually just using DiffEqJump directly.
  • Along those lines, the ReactionSystem model used 2*k[n] for reactions with involving reactants that had the same number of monomers in them, however, what you were plotting and matching to the theory was just using k[n] for such reactions! I switched the ReactionSystem to use k[n] too since that is what made it match the theoretical formulas you used. Please double check that this is all correct though! You can see the new figure I made in the assets folder.

Let me know after you've taken a look, and if you approve I'll merge.

@yewalenikhil65
Copy link
Contributor Author

OK, I updated it one more time. I'd be happy to merge it now. Some notes:

* You weren't actually using ModelingToolkit / Catalyst in your previous code. When you created the `MassActionJump` and passed it to `JumpProblem` you were actually just using `DiffEqJump` directly.

Yes, you are right. But i guess pushing reactions into for loop uses ModelingToolkit .

* Along those lines, the `ReactionSystem` model used `2*k[n]` for reactions with involving reactants that had the same number of monomers in them, however, what you were plotting and matching to the theory was just using `k[n]` for such reactions! I switched the `ReactionSystem` to use `k[n]` too since that is what made it match the theoretical formulas you used. Please double check that this is all correct though! You can see the new figure I made in the assets folder.

Again you are right.I plotted for just k[n] when I wrote the tutorial and uploaded that figure...But i have double checked using 2*k[n] and k[n]also. In both cases, multiplicative kernel doesn't match with analytical ..I do not know why!

Let me know after you've taken a look, and if you approve I'll merge.

We can merge it now. Only bit I am unsure is how to reason no-match of multiplicative kernel case, so i skipped commenting on it.

@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

Hmm, perhaps we should just use the additive kernel then and remove the other two?

@yewalenikhil65
Copy link
Contributor Author

Hmm, perhaps we should just use the additive kernel then and remove the other two?

Additive and constant kernels..Both are matching with analytical profiles..
I am not comfortable with removing multiplicative kernel even though its not matching. What do you think about calculation of the bulk volume of the system with which we are using in calculation of rate-constants ? Did we miss anything there ?

@yewalenikhil65
Copy link
Contributor Author

yewalenikhil65 commented Mar 3, 2021

Hey @isaacsas ..just realised something
This non-matching behaviour for 2*k[n] that you got is because of this 58a703e
I believe if you do add stoich construction as I did previously ,it matches in additive and constant kernel even if you use 2*k[n] as i mentioned earlier. Following part i mean

rx = [];              # empty-reaction vector
reactant_stoich = Array{Array{Pair{Int64,Int64},1},1}(undef, nr);
net_stoich = Array{Array{Pair{Int64,Int64},1},1}(undef, nr);
@time for n = 1:nr
    if (vᵢ[n] == vⱼ[n])    # checking the reactants
        push!(rx, Reaction(2*k[n], [ X[vᵢ[n]] ] ,[ X[sum_vᵢvⱼ[n]] ] ,[2],[1]));
        reactant_stoich[n] = [vᵢ[n] => 2];                                    # this
        net_stoich[n] = [vᵢ[n] => -2, sum_vᵢvⱼ[n] => 1];                 # this
    else
        push!(rx, Reaction(k[n], [ X[vᵢ[n]] , X[vⱼ[n]] ] ,[ X[sum_vᵢvⱼ[n]] ],
                                [1, 1],[1]));
        reactant_stoich[n] = [vᵢ[n] => 1 , vⱼ[n] => 1];                          # this
        net_stoich[n] = [vᵢ[n] => -1 , vⱼ[n] => -1 , sum_vᵢvⱼ[n] => 1];   # this
    end
end
rs = ReactionSystem(rx, t, X, k);
jumpsys = convert(JumpSystem, rs; combinatoric_ratelaws = true);
dprob = DiscreteProblem(jumpsys, u₀map, tspan, pₘₐₚ; parallel = true);
alg = Direct();                     stepper = SSAStepper();
mass_act_jump = MassActionJump(kₛ ,reactant_stoich, net_stoich);
jprob = @btime JumpProblem(dprob, alg ,mass_act_jump ,save_positions=(false, false));

I am confused why it is such though. If you remove this stoich construction, you don't get the match for 2*k[n] as you pointed

@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

The MassActionJump you are building is using k[n] though not 2*k[n] for those reactions, right? When you use the mass_act_jump none of the ReactionSystem code is even getting used so it is irrelevant what you set as the rate in the Reactions.

@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

Unfortunately, I haven't read this exact solution paper / method, so can't comment on why the multiplicative case does not match it.

@yewalenikhil65
Copy link
Contributor Author

The MassActionJump you are building is using k[n] though not 2*k[n] for those reactions, right? When you use the mass_act_jump none of the ReactionSystem code is even getting used so it is irrelevant what you set as the rate in the Reactions.

Ahh.. understood now.. In that case, 58a703e seems correct and usage of k[n] seems more correct. I just latexified reactions into ODEs and checked if it matches with equation in wiki page.Usage of k[n] is appropriate one as you have done.
I think we can leave multiplicative case for now then. and merge only additive and constant kernels case ?

@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

OK, can you remove the multiplicative case from the tutorial text and code? I'll then take a look and merge.

@yewalenikhil65
Copy link
Contributor Author

OK, can you remove the multiplicative case from the tutorial text and code? I'll then take a look and merge.

@isaacsas ..Done!

@isaacsas isaacsas merged commit 45869f3 into SciML:master Mar 3, 2021
@isaacsas
Copy link
Member

isaacsas commented Mar 3, 2021

All merged! Thanks!

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