Closed gdalle closed 4 months ago
@gdalle thanks for your additional comments. I will work on them and respond to your remarks next week. The annotations on the PDF file work great for me.
@gdalle Thanks again for your time to review our paper. Please find below my comments to the issues you raised.
It would also be great that you keep an eye at PR #404, where we'll finalize our revision. It would be easier if this time we could incorporate all your comments before merging the PR, such that when the paper goes to whedon it can be completed without yet another round of revisions and PR.
JuliaCon/proceedings-review#133
I have re-read the paper, overall the presentation has improved indeed! In particular, this time around I have finally understood the syncing aspect which makes your algorithm different. Thus I was able to focus a bit more on the mathematical part at the beginning.
That's great!
My main remaining issues are:
* The introduction of `Coevolve` still doesn't make it clear what is yours and what is theirs. I suggest introducing a new name, maybe CoevolveSync, since from what I understand this is your major contribution to an existing algorithm
Initially, my plan was to implement Coevolve
. Once I realized that it was possible to improve Coevolve
to run in sync with ODE solvers, I implemented the improved version which I coincidentally named CoevolveSynced
. However, during the PR process at the time, we came to the conclusion that it was redundant to have both Coevolve
and CoevolveSynced
in the same library because the latter supersedes the former. From a maintenance perspective, this strategy requires less effort and is less error-prone. In the PR, I also provide some evidence that the improved Coevolve
is a bit faster (and this was before we made improvements to the PR and the library in general).
At this point, it would be confusing for a user of the library to have the algorithm referenced as CoevolveSynced
in the paper and as Coevolve
in the library. Therefore, it would be better to emphasize and make it clear in the paper that our version of Coevolve
improves the original one. I tried to use Coevolve
in the paper only when referencing the original algorithm or the aggregator in the library.
The idea of running the original Ogata algorithm can be traced back even earlier than the COEVOLVE paper (Farajtbar et al 2017). The next reaction method from Gibson and Bruck (2000) shares some similarities with COEVOLVE. Furthermore, Daley and Vere-Jones (2003) already discussed the idea of synced algorithms after listing Ogata's modified thinning algorithm (Algorithm 7.5.IV). In page 272, they say (with the main challenge highlighted by me):
"Algorithm 7.5.IV can be extended to cover the situation where the evolution of the conditional hazard function depends on additional random processes, themselves evolving jointly with the given point process. The immediate requirements are for the existence of explicit algorithms for calculating the intensity and for finding local bounds L(·) and M (·) that take into account current and past values of the auxiliary variables. A deeper difficulty, however, relates to the need to simulate forward not only the point process but also the auxiliary variables on which it depends. For auxiliary variables that change only slowly, this may not be a serious handicap, but for longer-term predictions, a full model is needed from which the point process and auxiliary variables can be jointly simulated."
However, until our paper, we have not seen an algorithm implementation of such idea, which is where our main contribution lies.
I have modified a few parts of the introduction to reflect the above discussion.
* The section on inverse methods is rather confusing and could be shortened
The reason that the section on inverse method is longer is because it derives CHV simple and CHV full. These derivations are not directly available in the sources we mentioned, but are important for implementing the TPP simulations which we benchmark in the experiment section. It also sets the stage for the theoretical discussion that follows that section. Nevertheless, I tried to improve on the points you annotated. I hope that it makes the section easier to follow.
* In Algorithm 5 it would be nice to be able to compare with a naive queueing algorithm, to see the benefits of syncing
The naive "queuing" algorithms are the NRM and Tick. The benchmark section serves to compare the different implementation of queuing algorithms.
* On the easiest benchmark I don't understand why queueing methods perform poorly: are these instances 1-dimensional (in the mark space)?
The easiest benchmark are not unmarked, but they are constant rate processes. That means a lot of the tricks that were developed for thinning this type of processes make them faster than queueing. In the future, it would be a good idea to port these ideas to queueing algorithms.
I added a sentence to the paper on that.
* The comparison with Tick is great, I would also add brief references to other point process libraries in Python and R
Done.
You can find all my remarks as annotations on the PDF file. Is it okay as a format for exchanging remarks? Can you read them?
Below you find my comments on some of the annotations that I could not directly address in the paper (page and line numbers refer to the manuscript you annotated):
page 1, line 5: Maintained "also provides". It is short and conveys the equivalent idea that the library offers the advertised capability.
page 2, line 121: $T = - \ln(V) / \lambda \sim \exp(1)$ is correct. See this Wikipedia Section for a derivation.
page 2, line 156: Replace $t(0)$ with $\tilde{t}_{n-1}$. Here $0$ refers to the interval $\Delta \tilde{t}$. I have re-written the derivative with $\Delta \tilde{t}$ to make it more clear.
page 3, line 162: Remove reference to $\varphi$. It is hard to remove the notation for the flow because it plays an important role in the derivation of CHV full. Alternatively, I have expressed Equation 4.8 in terms of $\varphi$. I hope it makes the connection more clear.
page 3, line 179 (and other similar comments): Equation 4.2 is indeed the correct reference. I have added an extra equivalence in the equation in an attempt to make the problem more obvious.
page 5, line 343: It would require extra effort to show the difference as colored lines in the listed algorithms. It is also not just a matter of highlighting a few lines, but the algorithms have slight different steps. We try to highlight the difference in our discussion in the main text.
page 9, line 724: Indeed CHV (full or simple) was not implemented in JumpProcesses.jl. This is mentioned previously around line 670. Implementation of CHV in JumpProcesses.jl is current work in progress (see issue #291 and CHV branch).
PR #404 was merged to the paper branch. You can find the latest draft in JuliaCon/proceedings-review#133.
Thanks, the changes look good, in particular the clarifications about Coevolve! Here are a few typos I spotted but otherwise I'm okay with this version Zagatti et al. - 2023 - Extending JumpProcesses.jl for fast point process .pdf
That's great news!
I fixed the typos in PR #406. Once it gets merged we can close this issue.
https://github.com/JuliaCon/proceedings-review/issues/133
I have re-read the paper, overall the presentation has improved indeed! In particular, this time around I have finally understood the syncing aspect which makes your algorithm different. Thus I was able to focus a bit more on the mathematical part at the beginning.
My main remaining issues are:
Coevolve
still doesn't make it clear what is yours and what is theirs. I suggest introducing a new name, maybe CoevolveSync, since from what I understand this is your major contribution to an existing algorithmYou can find all my remarks as annotations on the PDF file. Is it okay as a format for exchanging remarks? Can you read them?