JuliaCon / proceedings-review

6 stars 1 forks source link

[REVIEW]: Extending JumpProcess.jl for fast point process simulation with time-varying intensities #133

Closed whedon closed 2 months ago

whedon commented 10 months ago

Submitting author: !--author-handle-->@gzagatti<!--end-author-handle-- (Guilherme Augusto Zagatti) Repository: https://github.com/SciML/JumpProcesses.jl Branch with paper.md (empty if default branch): juliacon2023 Version: v9.9 Editor: !--editor-->@matbesancon<!--end-editor-- Reviewers: !--reviewers-list-->@gdalle<!--end-reviewers-list-- Archive: 10.5281/zenodo.10786561

Status

status

Status badge code:

HTML: <a href="https://proceedings.juliacon.org/papers/886f7f58f837a2c2d7437254c024cb0e"><img src="https://proceedings.juliacon.org/papers/886f7f58f837a2c2d7437254c024cb0e/status.svg"></a>
Markdown: [![status](https://proceedings.juliacon.org/papers/886f7f58f837a2c2d7437254c024cb0e/status.svg)](https://proceedings.juliacon.org/papers/886f7f58f837a2c2d7437254c024cb0e)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@gdalle, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

  1. Make sure you're logged in to your GitHub account
  2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @matbesancon know.

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Review checklist for @gdalle

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Paper format

Content

whedon commented 10 months ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @gdalle it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/JuliaCon/proceedings-review) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/JuliaCon/proceedings-review:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

For a list of things I can do to help you, just type:

@whedon commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@whedon generate pdf
matbesancon commented 10 months ago

@whedon generate pdf from branch juliacon

whedon commented 10 months ago
Attempting PDF compilation from custom branch juliacon. Reticulating splines etc...
whedon commented 10 months ago

PDF failed to compile for issue #133 with the following error:

 Can't find any papers to compile :-(
whedon commented 10 months ago
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.17 s (554.4 files/s, 74814.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Julia                           68           1256            703           7287
Markdown                        13            601              0           2415
YAML                             8             12              8            183
TOML                             3              5              0             67
TeX                              1              0              0             13
-------------------------------------------------------------------------------
SUM:                            93           1874            711           9965
-------------------------------------------------------------------------------

Statistical information for the repository '0ca2f44339de62ccc342dbe0' was
gathered on 2023/08/28.
No commited files with the specified extensions were found.
matbesancon commented 10 months ago

@whedon generate pdf from branch juliacon2023

whedon commented 10 months ago
Attempting PDF compilation from custom branch juliacon2023. Reticulating splines etc...
whedon commented 10 months ago

PDF failed to compile for issue #133 with the following error:

 Can't find any papers to compile :-(
whedon commented 10 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

matbesancon commented 10 months ago

there we go

gdalle commented 10 months ago

thought we'd never get there :rofl:

matbesancon commented 9 months ago

same :grimacing:

matbesancon commented 9 months ago

@whedon add @mschauer as reviewer

whedon commented 9 months ago

OK, @mschauer is now a reviewer

mschauer commented 9 months ago

Disclaimer: I am collaborating with Chris on related topics, @matbesancon and I discussed shortly and decided that I will accept this review request.

matbesancon commented 9 months ago

@mschauer @gdalle thanks for agreeing to review, let us know if you need anything, indication, etc

whedon commented 9 months ago

:wave: @gdalle, please update us on how your review is going (this is an automated reminder).

mschauer commented 9 months ago

Can you turn on line numbers?

gzagatti commented 9 months ago

Here is the paper with line numbers to facilitate your review: paper.pdf. Hope the review is going well. Thanks for doing it!

gzagatti commented 9 months ago

@gdalle @mschauer thanks for reviewing the paper. I was wondering if you had time to advance with the review. I'm looking forward to addressing your feedback to get the paper published as soon as possible.

gdalle commented 9 months ago

I'm sorry I was completely swamped lately. I'm gonna try to take a first look tomorrow

mschauer commented 9 months ago

Some minor things, it helps me to get started with the paper.

gzagatti commented 9 months ago

@mschauer thanks for your comments.

You are conditioning on everything in $H_{t^-}$ , so the explanation in line 95 is off.

I didn't quite understand your comment here. In line 95, I am trying to explain in words the meaning of $p^\ast (t)$, so it's indeed the case that $p^\ast$ is conditioned on the history $H_{t^-}$.

Line 124 needs "independently"

I will add that.

Thinning is exact and root-finding methods are some less exact than that, so maybe change the section title in 4.1?

Noted. Just to make sure we're in the same page, root-finding methods should be exact in theory but in so far as numerical implementation goes they can never be exact. Is that right?

In (4.9) I use a Taylor polynomial on ...

That sounds like a promising avenue for automatically setting the upper boundary without user-input which is something we mentioned in the Conclusion. Could you point to the part of your package that has the PDMP sampler/dynamics and uses this trick? We could consider this idea in future developments of JumpProcesses.jl. We can add a discussion of your idea in the last section of the paper as future work. We already had the following:

[...] Second, the new aggregator depends on the user providing bounds on the jump rates as well as the duration of their validity. In practice, it can be difficult to determine these bounds a priori, particularly for models with many ODE variables. Moreover, determining such bounds from an analytical solution or the underlying ODEs does not guarantee their holding for the numerically computed solution (which is obtained via an ODE discretization), and so modifications may be needed in practice. A possible improvement would be for \texttt{JumpProcesses.jl} to determine these bounds automatically taking into account the derivative of the rates. Deriving efficient bounds require not only knowledge of the problem and a good amount of analytical work, but also knowledge about the numerical integrator.

mschauer commented 9 months ago

See https://github.com/mschauer/ZigZagBoomerang.jl/blob/691afe2200fa99d762b58f5c5fd7894cc9342294/src/not_fact_samplers.jl#L285

gzagatti commented 8 months ago

@gdalle @mschauer I would like to follow up with you on the review process. I am wondering if you had time to complete it, as I would like to incorporate all your suggestion and close this process as soon as we can.

Don't forget to complete the checklist below, so that the editors can proceed with the review process.

Again many thanks for taking the time!

Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest

Review checklist

Conflict of interest

Code of Conduct

General checks

  • [ ] Repository: Is the source code for this software available at the repository url?

  • [ ] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?

  • [ ] Authorship: Has the submitting author (@gzagatti) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

  • [ ] Installation: Does installation proceed as outlined in the documentation?

  • [ ] Functionality: Have the functional claims of the software been confirmed?

  • [ ] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

  • [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

  • [ ] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.

  • [ ] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).

  • [ ] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?

    • [ ] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [ ] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Paper format

  • [ ] Authors: Does the paper.tex file include a list of authors with their affiliations?

  • [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?

  • [ ] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

  • [ ] Page limit: Is the page limit for full papers respected by the submitted document?

Content

  • [ ] Context: is the scientific context motivating the work correctly presented?

  • [ ] Methodology: is the approach taken in the work justified, presented with enough details and reference to reproduce it?

  • [ ] Results: are the results presented and compared to approaches with similar goals?

gdalle commented 8 months ago

I'm sorry I completely overestimated my bandwidth with this one. Will get it done before the end of this month

gzagatti commented 7 months ago

@gdalle @mschauer I'd just like to follow up on the review since we just started a new month. I really hope that we can move this forward in the coming days and I appreciate your time!

gdalle commented 7 months ago

I'll start it today

gdalle commented 7 months ago

Can't edit the checklist so I'm copy-pasting it here

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Paper format

Content

gdalle commented 7 months ago

@whedon generate pdf from branch juliacon2023

whedon commented 7 months ago
Attempting PDF compilation from custom branch juliacon2023. Reticulating splines etc...
whedon commented 7 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

gzagatti commented 7 months ago

Thanks @gdalle for all your comments. Really appreciated! I've noticed that @ChrisRackauckas and @isaacsas have already responded to some of comments. I will work on all your comments tomorrow.

gdalle commented 7 months ago

With pleasure! Currently making my way through the paper itself now that I'm done with the docs

gzagatti commented 7 months ago

@gdalle and @mschauer many thanks again for your review.

SciML/JumpProcesses.jl#372 and PRs SciML/JumpProcesses.jl#373 address your feedback. Please let us know if there is any outstanding issues before we get them merged. Here is the latest version of the paper

Once you are satisfied that all your concerns have been addressed please help us by completing the checklist and endorsing our work for publication as quoted by the proceedings instructions:

The reviewers will do there work and read and constructively criticize your paper and the corresponding code repository. The idea is to bring your submission into the best shape possible. Once the reviews are in, you should address the raised points, e.g. by making corrections, clarifying things, or fixing bugs. The revised submission will then be reviewed (at least) once more by the reviewers until they endorse your work for publication.

It has been a pleasure working with you so far. I am looking forward to getting this completed before the end of this year.

gzagatti commented 6 months ago

@mschauer we haven't heard back from you since you initially made some preliminary comments back in September. We are currently reviewing the paper based on both of your suggestions but your position on whether you have completed the review is not clear. Could you please let us know if you have done so by sharing your review checklist with us? I hope that we can complete this review process as soon as possible to avoid any further delays. Many thanks for your help.

mschauer commented 6 months ago

Sorry, I want to let you know that I have to withdraw from this review unfortunately, I like the article though, it was nice reading it.

gzagatti commented 5 months ago

@whedon generate pdf from branch juliacon2023

whedon commented 5 months ago
Attempting PDF compilation from custom branch juliacon2023. Reticulating splines etc...
whedon commented 5 months ago

PDF failed to compile for issue #133 with the following error:

 Latexmk: This is Latexmk, John Collins, 17 Jan. 2018, version: 4.55.
Rule 'pdflatex': Rules & subrules not known to be previously run:
   pdflatex
Rule 'pdflatex': The following rules & subrules became out-of-date:
      'pdflatex'
------------
Run number 1 of rule 'pdflatex'
------------
------------
Running 'pdflatex  -recorder  "paper.tex"'
------------
===========Latexmk: Missing input file: 'mhsetup.sty' from line
  '! LaTeX Error: File `mhsetup.sty' not found.'
Latexmk: Missing input file: 'mhsetup.sty' from line
  '! LaTeX Error: File `mhsetup.sty' not found.'
Failure to make 'paper.pdf'
Collected error summary (may duplicate other messages):
  pdflatex: Command for 'pdflatex' gave return code 1
      Refer to 'paper.log' for details
Looks like we failed to compile the PDF
gzagatti commented 5 months ago

@whedon generate pdf from branch juliacon2023

whedon commented 5 months ago
Attempting PDF compilation from custom branch juliacon2023. Reticulating splines etc...
whedon commented 5 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

gzagatti commented 5 months ago

@gdalle we completed our review. Feel free to take another round of review.

To recap, we addressed all the points you raised in SciML/JumpProcesses.jl#372 and SciML/JumpProcesses.jl#373. We merged these PRs to the juliacon2023 branch. The latest compilation above refers to our latest draft.

Also, please find the version of the draft with line numbers for easier referencing.

Once you are satisfied that all your concerns have been addressed please help us by completing the checklist and endorsing our work for publication as described in the proceedings instructions.

It's been a pleasure working with you. Thanks again for all your feedback.

gzagatti commented 5 months ago

@whedon generate pdf from branch juliacon2023

whedon commented 5 months ago
Attempting PDF compilation from custom branch juliacon2023. Reticulating splines etc...
whedon commented 5 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

gzagatti commented 5 months ago

@gdalle please find above the latest version of our paper, which address your latest comments as discussed in SciML/JumpProcesses.jl#395.

gdalle commented 5 months ago

Thanks, I'll try to get to it next week

matbesancon commented 4 months ago

reminder @gdalle :)