Closed gzagatti closed 6 months ago
@gzagatti thanks! I will give this a look through so we can hopefully get it merged very soon. (Hopefully tonight or tomorrow.)
We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report. To ensure accuracy in future PRs, please see these guidelines. A quick fix for this PR: rebase it; your next report should be accurate.
Files with Coverage Reduction | New Missed Lines | % | ||
---|---|---|---|---|
src/aggregators/prioritytable.jl | 1 | 86.86% | ||
src/extended_jump_array.jl | 1 | 81.58% | ||
src/coupling.jl | 2 | 78.18% | ||
src/spatial/spatial_massaction_jump.jl | 3 | 85.45% | ||
<!-- | Total: | 7 | --> |
Totals | |
---|---|
Change from base Build 7091794717: | -0.3% |
Covered Lines: | 2297 |
Relevant Lines: | 2561 |
@gzagatti left you some comments.
Thanks @isaacsas.
Perhaps we should instead add a brief but fully explained mathematical discussion as requested in the review as the "getting started tutorial", and then link there to the concrete tutorials for people with different backgrounds. That would avoid needing to try to write a single tutorial that is accessible to all fields, while providing a single point that sends users to the tutorial that might work best for them.
Just to make sure I understand this part, before I start refactoring the "Getting Started" page. We want to keep this new page, but it needs some refactoring. Is that correct?
Next, are you suggesting that we just keep a high-level discussion and point to relevant tutorials (less work, we can keep some high-level equations) or that we include a full mathematical description for different audiences and then point to relevant tutorials (more work, since I feel I would be re-writing a lot of the equations that are already present in other tutorials).
I was suggesting not having any code on the "getting started" page, but instead having it direct users to the appropriate starting tutorials for their background (Poisson Processes, Stochastic Chem Kinetics, or your new Point Processes tutorial).
The review comment suggested adding the math right up front, so we could add such a paragraph in the index page or the getting started page, showing jump-diffusion equations mathematically, briefly explaining the terms, and then mentioning they include as special cases jump processes (temporal point processes), and PDMPs.
@isaacsas I have addressed most of your feedback and thanks for taking the time.
Regarding the "Getting Started", I got a bit stuck. I made a few changes to keep it simpler. But I think removing all the code will sort of make it redundant with the index since the index already contains some brief explanation of the maths and links to all tutorials.
My goal with "Getting Started" was to go through the motion of setting the problem up and solving with little else. It even breaks it down on the same 3 steps as in DifferentialEquations and discuss the very basic constructs in the library. For instance, the reviewer was confused with terms like aggregator and stepper. This page provides brief explanation of them in context.
I feel that a beginner would get a bit lost with the remaining tutorials because they are much longer and address more specific models.
Feel free to remove the "Getting Started" tutorial if you still not convinced. Or even to refactor it to your liking.
Following @gdalle review of our JuliaCon paper (thank you very much!), I have modified the documentation to address all his concerns.
The goal was to make the documentation more accessible to beginners and those coming from point process theory. I have added two new sections "Getting Started with JumpProcesses in Julia" and "Temporal Point Processes (TPP) with JumpProcesses".
Please see my comments to all the points raised by the reviewer.
From Issue #365.
I added a paragraph to situate a new user. I have updated the list of links and split it into two lists, one for tutorials and one for references and APIs.
The front page also guides the user to a new page "Getting Started with JumpProcesses in Julia" to align with the DifferentialEquations documentation which also contains a similar page.
I have also aligned the sidecar with the DifferentialEquations documentation to ensure that a user familiar with that library documentation will also be familiar with JumpProcesses documentation.
I have rephrased the paragraph where we introduce the aggregator to provide more intuition on this type of object. Also, see "Getting Started with JumpProcesses in Julia" for more accessible discussion on aggregators and steppers.
This paragraph was indeed very confusing. I have re-written it to make it more clear. The main point is that birth and death processes are orthogonal to the state variable
u
. The death process does not affect the birth process, because the later only obeys seasonal fluctuation. In other words, it is not because a lot of people are dying that agents will change their fertility behavior.I have removed the dependency on the global variable.
H
is now part of the parametersp
that are used to initialize the problem.I have added some notes to clarify that we are only saving the state variable
u
at end of section "Reducing Memory Use: Controlling Saving Behavior"I have added a beginner-level discussion below the equation.
This is just a coincidence, since all paths are stochastic they might actually look very different from simulation to simulation. In any case, I have added a discussion below the plots to explain what the different problem definitions bring to the simulated paths.
I have also reduced the complexity of the problems by removing the copied jump. It doesn't really add much to the overall goal of the tutorial and might be confusing. The main goal of the tutorial is to show how JumpProcesses can be used with discrete problems, ODEs, or SDEs.
I have added a "Getting Started with JumpProcesses in Julia" to align with the DifferentialEquations documentation which also contains a similar page. A concise tutorial in the same style as the parent library should make JumpProcesses more accessible.
That is now discussed in "Getting Started with JumpProcesses".
The distinction should be made clear and accessible in "Getting Started with JumpProcesses".
Thanks!
From Issue #370.
I have added a complete new tutorial on temporal point processes (TPP) with JumpProcesses. It guides someone familiar with TPP theory on how to use JumpProcesses to simulate TPPs.
As an added bonus, I am using PointProcess to define
SciMLPointProcess
which inheritsAbstractPointProcess
whose API contained many TPP constructs.The tutorial builds
SciMLPointProcess
while apply it to a concrete case the marked Hawkes process.I have implemented all the API from PointProcess except for fitting.
Regarding TPP parameter fitting, we cannot always derive an analytical estimator via maximum likelihood for TPPs. Therefore, it is beyond the scope of the tutorial to implement such method. I have added a discussion of this fact at the end of the tutorial.
Fix issues #365, #370 and JuliaCon/proceedings-review#133.