-
Notifications
You must be signed in to change notification settings - Fork 528
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Paper: RoughPy #904
Paper: RoughPy #904
Conversation
Curvenote Preview
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly I would like to apologise to the authors as the subject of this paper is outwith my wheelhouse, and as such have provided minor comments where I believe this will help the manuscript.
Review:
This manuscript describes RoughPy, a Python package for viewing streams of data through the lens of rough paths and using the tools and techniques of signatures. The manuscript is well-written and clear, with good accompanying documentation at https://roughpy.org/.
The codebase at https://github.com/datasig-ac-uk/RoughPy seems complete with well-written documentation, testing (including automated testing through Github Actions), and regular releases. There are currently no outstanding issues (https://github.com/datasig-ac-uk/RoughPy/issues) that cause concern, however, it would be nice to merge in datasig-ac-uk/RoughPy#91 or an equivalent. The inclusion of an issue template is noted and appreciated.
I am personally unsure of the reasoning behind the tests/build (windows-2019)
failure in the CI:
FAILED tests/streams/test_lie_increment_path.py::test_from_array_width_3_2_increments - AssertionError: { 1() 3/3(1) 7(2) 1(3) } != { 1() 3(1) 7(2) 1(3) }
while ubuntu-latest
/macos-11
pass, but would encourage the authors to investigate a fix to this issue.
Very interesting paper and package. Could you provide a link to more detailed examples if they are too complex to fully fit into the text? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a mathematician so I confess most of this paper went over my head, but from what I could tell it was well written. For me it would be helpful if there were more of a link between the very high-level applications (e.g. detecting interference) and the very simple example (computing the signature of the word 'scipy'). I know the authors point to the documentation for more information, but perhaps a slightly longer example would be helpful. However, I recognize I am probably not the intended audience for this paper, so it's fine to pass on this suggestion.
Minor comments:
- Typo, 'On simple'
- What does it mean "speed at which events occur"? Are we talking about time series, text streams, event time stamps, etc.?
- Typo, 'need to be done' (no period)
- What is 'the sequel'?
- Typo, 'unversal'
- Typo, 'In order to property'
Thanks so much for your comments. I'm looking to fix the issues that you highlighted. @cdlindsey I cited the Datasig website for the more complex examples, but I guess this wasn't sufficiently clear. I will adjust my wording to make this more clear. @cliffckerr Part of the purpose of RoughPy is that non-mathematicians can make use of the complicated machinery that exists below the surface, so in some way you are exactly part of the target audience for the package. Unfortunately, rough analysis is not sufficiently well-known outside of the mathematics community that I can reference it actually defining everything. We do have a bit of space to play with in this paper, so I can certainly try to explain the connection between the example and the more involved applications. "In the sequel" is used to mean "the rest of the paper". I'm now realizing that it might not be so ubiquitous. |
OK, I believe I have addressed all the issues raised by the reviewers, and a few additional things I noticed too. This should be my final version. |
If you are creating this PR in order to submit a draft of your paper, please name your PR with
Paper: <title>
. An editor will then add apaper
label and GitHub Actions will be run to check and build your paper.See the project readme for more information.
Editor: Charles Lindsey @cdlindsey
Reviewers: