Spatial constant rate jump #343

Vilin97 · 2023-09-03T04:42:37Z

Add spatial constant rate jump support.

NB: I have not implemented flattening for constant rate jumps, because it's unclear how to flatten constant rate jumps.

Implementation details:
In order to use constant rate jumps, the user is expected to pass in a JumpSet and two dependency graphs (reaction-reaction and reaction-species). The syntax is as follows:

rate1(u,p,t,site) = ...
rate2(u,p,t,site) = ...
affect1!(integrator,site) = ...
affect2!(integrator,site) =...
crjumps = JumpSet([ConstantRateJump(rate1, affect1!), ConstantRateJump(rate2, affect2!)])
# define prob, hopping_constants, spatial_system, and dependency_graphs
jump_prob = JumpProblem(prob, NSM(), crjumps, hopping_constants = hopping_constants,
            spatial_system = spatial_system , save_positions = (false, false), dep_graph = dep_graph, jumptovars_map = jumptovars_map)

Just use ConstantRateJump with `rate(u,p,t,site)`.

…e jumps.

Conflicts: src/spatial/reaction_rates.jl test/spatial/reaction_rates.jl

coveralls · 2023-09-09T04:37:11Z

Pull Request Test Coverage Report for Build 6180688297

31 of 38 (81.58%) changed or added relevant lines in 7 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.1%) to 89.981%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/problem.jl	2	3	66.67%
src/spatial/directcrdirect.jl	3	6	50.0%
src/spatial/nsm.jl	3	6	50.0%

Totals
Change from base Build 6167199515:	0.1%
Covered Lines:	2326
Relevant Lines:	2585

💛 - Coveralls

…to spatial-constant-rate-jump-2

isaacsas

Instead of passing the site into the affect and rate functions, we could store it in the aggregator and make an interface for users to access it via the integrator (which stores the aggregator). Something like get_current_site(integrator). Then this would work with old-style constant rate jump signatures. I don't know if we want that though.

The other big issue is I think there are going to be type stability issues without using FunctionWrappers on the rate and affect functions (since each function is its own type, every crj will be a different type).

isaacsas · 2023-12-29T23:54:59Z

src/spatial/directcrdirect.jl

@@ -39,6 +39,9 @@ function DirectCRDirectJumpAggregation(nj::SpatialJump{J}, njt::T, et::T, rx_rat

    # a dependency graph is needed
    if dep_graph === nothing
+        if length(rx_rates.cr_jumps) != 0


Suggested change

if length(rx_rates.cr_jumps) != 0

if !isempty(rx_rates.cr_jumps)

isaacsas · 2023-12-29T23:55:30Z

src/spatial/directcrdirect.jl

@@ -54,6 +57,9 @@ function DirectCRDirectJumpAggregation(nj::SpatialJump{J}, njt::T, et::T, rx_rat
    end

    if jumptovars_map === nothing
+        if length(rx_rates.cr_jumps) != 0


Suggested change

if length(rx_rates.cr_jumps) != 0

if !isempty(rx_rates.cr_jumps)

isaacsas · 2023-12-29T23:56:21Z

src/spatial/nsm.jl

@@ -32,6 +32,9 @@ function NSMJumpAggregation(nj::SpatialJump{J}, njt::T, et::T, rx_rates::RX, hop

    # a dependency graph is needed
    if dep_graph === nothing
+        if length(rx_rates.cr_jumps) != 0


same as above

isaacsas · 2023-12-29T23:56:30Z

src/spatial/nsm.jl

@@ -47,6 +50,9 @@ function NSMJumpAggregation(nj::SpatialJump{J}, njt::T, et::T, rx_rates::RX, hop
    end

    if jumptovars_map === nothing
+        if length(rx_rates.cr_jumps) != 0


same as above

isaacsas · 2023-12-29T23:58:18Z

src/spatial/reaction_rates.jl

 """

 ### spatial rx rates ###
-struct RxRates{F, M}
+struct RxRates{F, M, C}


For future consideration, do you know if storing the jumps in RxRates instead of within the aggregator directly results in an extra layer of indirection when accessing them (i.e. an extra pointer call)? If so, we might want to consider moving them up a level if that shows any performance benefit.

isaacsas · 2023-12-29T23:59:40Z

src/spatial/reaction_rates.jl

 end
+RxRates(num_sites::Int, ma_jumps::M) where {M<:AbstractMassActionJump} = RxRates(num_sites, ma_jumps, ConstantRateJump[])
+RxRates(num_sites::Int, cr_jumps::C) where {C} = RxRates(num_sites, SpatialMassActionJump(), cr_jumps)


Suggested change

RxRates(num_sites::Int, cr_jumps::C) where {C} = RxRates(num_sites, SpatialMassActionJump(), cr_jumps)

RxRates(num_sites::Int, cr_jumps::C) where {C} = RxRates(num_sites, SpatialMassActionJump(), cr_jumps)

I'd specify a type for C here or not include C at all. Generally a generic type like this would be suggested to just be dropped since it isn't being used (it may even give a warning in tests).

isaacsas · 2023-12-30T00:05:39Z

src/spatial/reaction_rates.jl

+            cr_jump = rx_rates.cr_jumps[rx - get_num_majumps(ma_jumps)]
+            set_rx_rate_at_site!(rx_rates, site, rx, cr_jump.rate(u, integrator.p, integrator.t, site))


This isn't type stable is it? That is why we split the ConstantRateJump rates and affects in the non-spatial solvers and wrap them inside FunctionWrappers.

isaacsas · 2023-12-30T00:05:50Z

src/spatial/reaction_rates.jl

+        cr_jump = rx_rates.cr_jumps[rx - get_num_majumps(rx_rates.ma_jumps)]
+        cr_jump.affect!(integrator, site)


This isn't type stable is it? That is why we split the ConstantRateJump rates and affects in the non-spatial solvers and wrap them inside FunctionWrappers.

Vilin97 added 5 commits September 2, 2023 22:28

Create SpatialConstantRateJump.

aa9edad

Hook up spatial constant rate jumps to RxRates.

49a6b49

Add constant rate jump support to RxRates and utils.jl.

964afaf

Add spatial constant rate jump to one unit test.

443dca3

Add RxRates constructor from constant rate jumps only.

eddb7bf

Vilin97 requested a review from isaacsas September 3, 2023 04:42

Vilin97 added 4 commits September 3, 2023 00:45

Remove SpatialConstantRateJump.

d0115c5

Just use ConstantRateJump with `rate(u,p,t,site)`.

Remove an include.

3dc9596

Add spatial ConstantRateJump to ABC test.

666d2cc

Add constant rate jump problems to the ABC.jl test.

f0997cd

Vilin97 changed the title ~~[WIP] Spatial constant rate jump 2~~ Spatial constant rate jump 2 Sep 3, 2023

Add an error message that flattening is not supported for costant rat…

0614fd9

…e jumps.

Vilin97 marked this pull request as ready for review September 3, 2023 06:03

Vilin97 added 2 commits September 4, 2023 19:43

Merge branch 'master'

9034abe

Conflicts: src/spatial/reaction_rates.jl test/spatial/reaction_rates.jl

Change empty spatial massaction jump to use concrete types.

ea26ce2

Vilin97 changed the title ~~Spatial constant rate jump 2~~ Spatial constant rate jump Sep 12, 2023

Vilin97 added 3 commits September 11, 2023 21:39

Fix two typos.

6f1ca89

Merge branch 'master' of https://github.com/SciML/JumpProcesses.jl in…

865aa69

…to spatial-constant-rate-jump-2

Merge branch 'master' of https://github.com/SciML/JumpProcesses.jl in…

5c97a63

…to spatial-constant-rate-jump-2

Vilin97 mentioned this pull request Nov 9, 2023

Including global variables that participate in MassActionJumps in spatial systems #364

Open

isaacsas requested changes Dec 30, 2023

View reviewed changes

Vilin97 marked this pull request as draft January 25, 2024 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spatial constant rate jump #343

Spatial constant rate jump #343

Vilin97 commented Sep 3, 2023 •

edited

coveralls commented Sep 9, 2023 •

edited

isaacsas left a comment

isaacsas Dec 29, 2023

isaacsas Dec 29, 2023

isaacsas Dec 29, 2023

isaacsas Dec 29, 2023

isaacsas Dec 29, 2023

isaacsas Dec 29, 2023

isaacsas Dec 30, 2023

isaacsas Dec 30, 2023

	if length(rx_rates.cr_jumps) != 0
	if !isempty(rx_rates.cr_jumps)

	RxRates(num_sites::Int, cr_jumps::C) where {C} = RxRates(num_sites, SpatialMassActionJump(), cr_jumps)
	RxRates(num_sites::Int, cr_jumps::C) where {C} = RxRates(num_sites, SpatialMassActionJump(), cr_jumps)

		cr_jump = rx_rates.cr_jumps[rx - get_num_majumps(ma_jumps)]
		set_rx_rate_at_site!(rx_rates, site, rx, cr_jump.rate(u, integrator.p, integrator.t, site))

		cr_jump = rx_rates.cr_jumps[rx - get_num_majumps(rx_rates.ma_jumps)]
		cr_jump.affect!(integrator, site)

Spatial constant rate jump #343

Are you sure you want to change the base?

Spatial constant rate jump #343

Conversation

Vilin97 commented Sep 3, 2023 • edited

coveralls commented Sep 9, 2023 • edited

Pull Request Test Coverage Report for Build 6180688297

💛 - Coveralls

isaacsas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vilin97 commented Sep 3, 2023 •

edited

coveralls commented Sep 9, 2023 •

edited