search

SciML / JumpProcesses.jl

Build and simulate jump equations like Gillespie simulations and jump diffusions with constant and state-dependent rates and mix with differential equations and scientific machine learning (SciML)
https://docs.sciml.ai/JumpProcesses/stable/
Other
135 stars 35 forks source link

Complete 2nd rebuttal, fix #402 #404

Closed gzagatti closed 4 months ago

gzagatti commented 4 months ago

This PR is the second round of rebuttal. It fixes #402.

Please also see my responses to the reviewer in the original issue.

This time I have made all my changes using the \comment macro to make it easier to spot all my changes in magenta in the compiled PDF. Once we are satisfied with all the changes. I will remove the macro and leave only the text. I hope that it makes it easier to review it that way.

cc: @gdalle

gzagatti commented 4 months ago

Find the compiled paper here for easier reference.

isaacsas commented 4 months ago

LGTM. Should I merge?

One minor comment, if you wanted to add a sentence speculating on why NRM/Coevolve don't offer the top performance on constant rate problems, you could say that it is due to the cost of updating their underlying indexed priority queue data structure which stores the next event times. A table-based data-structure would be expected to be more competitive this is the Sanft-Othmer paper, and I suspect a real cache-optimized indexed priority queue would give very good performance (but perhaps not beating RSSACR on the largest problems). The latter would be a great project actually that if as performant as I expect could be a J. Chem. Phys. paper...

gzagatti commented 4 months ago

Thanks for the quick turnaround! Let me add your minor comment and remove the comment markup before you merge.

gzagatti commented 4 months ago

Draft with minor comment added.

I will now remove the comment markup and let you know once it is good to merge.

isaacsas commented 4 months ago

The highlighting makes it much easier to quickly see what has changed!

gzagatti commented 4 months ago

Now, it's good to merge.