[REVIEW]: Lyapunov Cycle Detector (LCD.jl): A new tool for the study of basins of attraction induced by rational maps

whedon commented 7 months ago

Submitting author: !--author-handle-->@valvarezapa@lucaferranti<!--end-editor-- Reviewers: @lbenet, @rpgowers Archive: Pending

Status

Status badge code:

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

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

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

Make sure you're logged in to your GitHub account
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 @lucaferranti 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 @lbenet

Conflict of interest

[x] As the reviewer I confirm that I have read the JuliaCon conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

[x] I confirm that I read and will adhere to the JuliaCon code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Authorship: Has the submitting author (@valvarezapa) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[ ] Functionality: Have the functional claims of the software been confirmed?
[x] 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

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] 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

[x] Authors: Does the paper.tex file include a list of authors with their affiliations?
[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?
[x] Page limit: Is the page limit for extended abstracts respected by the submitted document?

Content

[x] Context: is the scientific context motivating the work correctly presented?
[x] Methodology: is the approach taken in the work justified, presented with enough details and reference to reproduce it?
[x] Results: are the results presented and compared to approaches with similar goals?

Review checklist for @rpgowers

Conflict of interest
[x] As the reviewer I confirm that I have read the JuliaCon conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

[x] I confirm that I read and will adhere to the JuliaCon code of conduct.

General checks

[x] Repository: Is the source code for this software available at the repository url?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Authorship: Has the submitting author (@valvarezapa) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

[x] 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 extended abstracts 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?

whedon commented 7 months ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @lbenet, @rpgowers 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:

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

watching

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

whedon commented 7 months ago

Failed to discover a Statement of need section in paper

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:

whedon commented 7 months ago

Wordcount for paper.tex is 523

whedon commented 7 months ago

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.04 s (354.5 files/s, 129418.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TeX                              8            222            167           2177
Julia                            1            168            448            996
Markdown                         1             35              0             56
Jupyter Notebook                 1              0            357             53
Ruby                             1              7              3             37
YAML                             1              0              0             20
-------------------------------------------------------------------------------
SUM:                            13            432            975           3339
-------------------------------------------------------------------------------

Statistical information for the repository '9d7a0420dd7e18b72ff2cc79' was
gathered on 2024/02/13.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Víctor Álvarez Apari             3            94             47          100.00

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Víctor Álvarez Apari         47           50.0          0.0                6.38

whedon commented 7 months ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- None

MISSING DOIs

- 10.1137/141000671 may be a valid DOI for title: Julia: A fresh approach to numerical computing
- 10.1016/j.topol.2023.108578 may be a valid DOI for title: Algorithms for computing basins of attraction associated with a rational self-map of the Hopf fibration based on Lyapunov exponents
- 10.32513/tbilisi/1528768904 may be a valid DOI for title: Plotting basins of end points of rational maps with Sage

INVALID DOIs

- None

lucaferranti commented 7 months ago

@lbenet @rpgowers thank you for volunteering to review.

You can find your review checklist at the top of this issue. As you go through the checklist, you can either comment here your observations or open directly issues in the software repository.

If you have any questions, let me know

whedon commented 7 months ago

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

whedon commented 7 months ago

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

lbenet commented 7 months ago

Sorry for the delay. I'm begining right now to look into this. I hope to finish by the end of the week...

lucaferranti commented 7 months ago

No problem at all @lbenet ! The bot sends an automatic reminder every 2 weeks, but you are not late :)

lbenet commented 7 months ago

First, apologies for the delay to send this review.

The paper is a readable and concise extended abstract, describing the authors package, which is rather specilized in the study of the basins of attraction of rational complex maps, without a priori requiring the computation of the fixed-points. I find the paper clear, it includes references which may address some of the checklist points that I left unchecked, such as the details and proofs of the methodology. While I am in favor of an eventual acceptance, I think the repo and the paper need some improvements to make the life easier to any potential user of the package; some concrete points are listed below. This is the reason that I recommend to make minor modifications in the paper and the repo.

Very important: there is no Project.toml file, so it is not obvious what dependencies are required (Polynomials, PyPlot and Colors); this point must be addressed by the author, since it complicates installing the package.
The instructions to install the package (in the README.md file) are outdated and do not work. The author should include a clause in the paper stating that the package is not in the General Registry, and include how to install it. These instructions should also be included in the README.md file, as well as in the LCD Quick User Guide.ipynb file. (I managed to clone locally the repo, and after installing its dependencies one by one I could load the package with include; the fact that there is no Project.toml stops Julia from installing it.)
There is no documentation of the package as such, which for this type of submission may be excused. Yet, there is a Jupyter notebook, LCD Quick User Guide.ipynb, which is very useful but outdated, and among other is aimed to illustrate many aspects of the package usage. I suggest to add a link to that file in the README.md file, and also update it as needed, so it can be used. Among other, that file clarifies what coefnum and coefden represent in the paper, and how to constructed them; in the paper these quantities are only mentioned with no details.
Along the same lines, the author should add in the paper, perhaps as the figure captions of the figures included, the coefficients used to produce those figures, which incidentally are not mentioned in the paper. Importantly, I could not produce the image of the basin of attraction for $z^3-1$ using the command in the paper; I only obtained a uniform grey box; the same occurs following the LCD Quick User Guide. This may be due to a limitation of something I didn't install, which can be solved by making explicitly what is truly needed.
The README.md file in the repo, which has lots of details, includes no references and seems outdated; this should be easily fixed. There, there are several mentions to the preprint "Mathematical Framework of LCD" (which I guess is Ref [2] in the paper), and the reader is invited to check it for details, without a link nor a proper reference. Since this file has not been modified for two years, I strongly suggest to update it, and consider updating Reference [2] in the paper, which is hopefully published by now.
There are no tests for the package, which again might be excused; yet, I strongly recommend to include at least some. Easy are the fixed points simple complex polynomials. More elaborated tests certainly be appreciated.
Regarding the paper itself, there is a typo: it is written "forming this calculations" and should be "forming these calculations". Also, I suggest to rephrase the clause "Consider the infinity point and work comfortably with it." in a more appropriete mathematical way... (Is "infinity" a point in the real line or in the complex plane?)

rpgowers commented 6 months ago

I started looking into this just over a week ago and have returned to it again recently. Review should be forthcoming over the next few days.

lucaferranti commented 6 months ago

(ignore the next message, it's to test the new infrastructure)

lucaferranti commented 6 months ago

Hello @editorialbot

lucaferranti commented 6 months ago

Hello @editorialbot

rpgowers commented 6 months ago

Here is my review for the article and repository of LCD.jl:

The paper is a succinct and readable article with a clear description and diagrams. The author outlines clearly the problem to be solved and provides useful visual diagrams showing example outputs from the package.

There are three main aspects which stand out in the article which could be improved. The first is that the example figures given do not have the arguments for coef_num and coef_den given. Provided that this can be done concisely, giving explicit values for the arguments would help the reader understand how to use the package. Secondly, the list of references seems out of date. The preprint is referenced rather than the published journal article of the same name in "Topology and its Applications" in 2023. Finally, the authors could spend a sentence to explain to the general reader that by "indeterminations" (which I took as "indeterminacies") that this term means a zero or near-zero denominator. This is because the latter concept of small denominators will be more widely understood.

Regarding the software repository, the installation instructions as described do not work. For example, it should read Pkg.add(url = "https://github.com/valvarezapa/LCD.jl.git), although even this change is insufficient to install the package correctly. Furthermore, the list of dependencies is not given and there is no Project.toml file.

Once one manages a workaround for the installation, the functions provided do run and there are examples given in the source code, which is welcome. The code is also well-commented and almost all (there were a few exceptions!) the examples contained run without error and some nice-looking plots were generated. The inclusion of an introductory jupyter notebook is also appreciated. However, here too there is scope for improvement. The main one being to implement automated tests. Second, the author should give could which generates the plots in the Examples folder, especially since these diagrams are the selling point of this package.

Overall this is an interesting package which solves a clear problem. Further refinements to the repository would help make it more accessible and attractive to other Julia users in the mathematical community.

rpgowers commented 6 months ago

I would also do the checklist now, but I cannot seem to edit it. I have gone through all the points in the checklist and know which boxes can be ticked off.

lucaferranti commented 6 months ago

@rpgowers @lbenet thank you for the review!

@valvarezapa you can start working on the points highlighted by the reviewers. Let me or the reviewers know if you have further questions

lucaferranti commented 6 months ago

@editorialbot commands

editorialbot commented 6 months ago

Hello @lucaferranti, here are the things you can ask me to do:


# List all available commands
@editorialbot commands

# Get a list of all editors's GitHub handles
@editorialbot list editors

# Adds a checklist for the reviewer using this command
@editorialbot generate my checklist

# Set a value for branch
@editorialbot set juliacon-paper as branch

# Reject paper
@editorialbot reject

# Withdraw paper
@editorialbot withdraw

# Invite an editor to edit a submission (sending them an email)
@editorialbot invite @(.*) as editor

# Run checks and provide information on the repository and the paper file
@editorialbot check repository

# Check the references of the paper for missing DOIs
@editorialbot check references

# Generates the pdf paper
@editorialbot generate pdf

# Accept and publish the paper
@editorialbot accept

# Update data on an accepted/published paper
@editorialbot reaccept

# Generates a LaTeX preprint file
@editorialbot generate preprint

# Get a link to the complete list of reviewers
@editorialbot list reviewers

lucaferranti commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

:warning: An error happened when generating the pdf.

xuanxu commented 6 months ago

@editorialbot generate pdf

editorialbot commented 6 months ago

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

lucaferranti commented 6 months ago

@rpgowers with the new app you should be able to do

@editorialbot generate my checklist

to get your checklist.

rpgowers commented 6 months ago

Review checklist for @rpgowers

Conflict of interest

[x] I confirm that I have read the JuliaCon conflict of interest policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JCon for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JuliaCon code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/valvarezapa/LCD?
[x] License: Does the repository contain a plain-text LICENSE or COPYING file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@valvarezapa) 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?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] 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

[x] 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.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality 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

[x] Authors: Does the paper.tex file include a list of authors with their affiliations?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Content

[x] Context: is the scientific context motivating the work correctly presented?
[x] 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?

rpgowers commented 6 months ago

@rpgowers with the new app you should be able to do
@editorialbot generate my checklist
to get your checklist.

Ah great, I managed to generate a checklist and filled it in now thanks

lucaferranti commented 5 months ago

Hi @valvarezapa :wave:,

how is it going? Have you started addressing the reviewers' comments? Any estimate on the timeline?

valvarezapa commented 5 months ago

Hi!

Apologies for the delay. I have not started yet with this, but I find the reviewers' comments very helpful and accurate. I am sure that their suggestions and comments will lead to a significant improvement both in the paper and the repository, and for that I am deeply grateful. I will start addressing this as soon as I can (and as soon as my other responsibilities allow me). I estimate that by the end of May I will be able to start working on this.

Again, thank you for your patience and thanks to the reviewers. Best, Víctor

El jue, 25 abr 2024 a las 18:30, Luca Ferranti @.***>) escribió:

Hi @valvarezapa https://github.com/valvarezapa 👋,

how is it going? Have you started addressing the reviewers' comments? Any estimate on the timeline?

— Reply to this email directly, view it on GitHub https://github.com/JuliaCon/proceedings-review/issues/140#issuecomment-2077697526, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYNK6LHO6D66H3HXIMQVQXDY7EVQ5AVCNFSM6AAAAABDGSI66OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXGY4TONJSGY . You are receiving this because you were mentioned.Message ID: @.***>

lucaferranti commented 3 months ago

Hi @valvarezapa :wave: , any updates on addressing the reviewers' comments?

valvarezapa commented 3 months ago

Hi!

I'm really sorry for the delay. I hope to be able to address the reviewer's comments in the following couple of weeks. I will keep you updated.

Thank you for everything. Best, Víctor

El lun, 17 jun 2024 a las 11:13, Luca Ferranti @.***>) escribió:

Hi @valvarezapa https://github.com/valvarezapa 👋 , any updates on addressing the reviewers' comments?

— Reply to this email directly, view it on GitHub https://github.com/JuliaCon/proceedings-review/issues/140#issuecomment-2172820776, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYNK6LH562TTWKHX7SOVQ2LZH2SE5AVCNFSM6AAAAABDGSI66OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZSHAZDANZXGY . You are receiving this because you were mentioned.Message ID: @.***>

valvarezapa commented 2 months ago

Hi!

First, apologies for the delay. I have addressed the reviewer's comments and updated the GitHub repository accordingly. I hope everything is correct now. I would like to thank the reviewers again, since their generous comments have improved both the article and the repository. I also want to thank Luca for his patience and his work. In the following days I'll try to include some tests, which hopefully will ease the set up process of the package.

Thank you again for your time. Best, Víctor

El vie, 21 jun 2024 a las 12:21, Víctor Álvarez Aparicio (< @.***>) escribió:

Hi!

I'm really sorry for the delay. I hope to be able to address the reviewer's comments in the following couple of weeks. I will keep you updated.

Thank you for everything. Best, Víctor

El lun, 17 jun 2024 a las 11:13, Luca Ferranti @.***>) escribió:

Hi @valvarezapa https://github.com/valvarezapa 👋 , any updates on addressing the reviewers' comments?

— Reply to this email directly, view it on GitHub https://github.com/JuliaCon/proceedings-review/issues/140#issuecomment-2172820776, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYNK6LH562TTWKHX7SOVQ2LZH2SE5AVCNFSM6AAAAABDGSI66OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZSHAZDANZXGY . You are receiving this because you were mentioned.Message ID: @.***>

lucaferranti commented 2 months ago

Thank you @valvarezapa for the updates!

@lbenet, @rpgowers please take a second look at the update paper and repository, update the checklist accordingly and let the author know if any further changes are required

lbenet commented 2 months ago

@editorialbot generate pdf

editorialbot commented 2 months ago

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

rpgowers commented 1 month ago

Apologies for taking a while to look back at this. I have started looking at the updated repository now. Regarding the installation instructions, and while the line:

add https://github.com/valvarezapa/LCD

does now install the package, it does not work with the following line:

using LCD.jl

This seems to be because Julia thinks the package is called LyapunovCycleDetector, since using LyapunovCycleDetector gives the error:

ERROR: package `LyapunovCycleDetector` did not define the expected module `LyapunovCycleDetector`, ...

I believe this should be fixable by simply renaming LyapunovCycleDetector.jl to LCD.jl (and making sure to rename the include statement in src/Examples.jl).

Will comment further on other aspects of the updated repository soon, this is just the most obvious (and hopefully easily fixable) one I have found.

rpgowers commented 1 month ago

The paper references have now been updated, so I have ticked that item now. The concepts and the example given in the paper are also now easier to understand.

rpgowers commented 1 month ago

The new Examples.jl file is appreciated and I was able to execute the functions there successfully. The Jupyter notebook is also very much appreciated.

I would ask to ensure that all the plots in the Examples directory have a corresponding equation in the Examples.jl file. I think that most of the cases do, but I believe many of the Julia set figures do not.

The readme reads better and I appreciate the updates here. One minor issue is that the urls to the papers seem to be combined into single links (e.g. [https://doi.org/10.1016/j.topol.2023.108578])(https://doi.org/10.1016/j.topol.2023.108578) ), which should be fixed.

A comment on the distinction between your method and the approach used in DynamicalSystems.jl (see: https://juliadynamics.github.io/DynamicalSystems.jl/previews/PR156/chaos/basins/) to find basins of attraction would also be appreciated. There is some overlap on what is trying to be achieved, however the approaches used seem to be very different. Pointing this out briefly in the paper and/or Readme would help users to better understand the strengths and best applications of the approach you present.

JuliaCon / proceedings-review