Joss-reviews: [REVIEW]: TaylorSeries.jl: Taylor expansions in one and many variables in Julia

Created on 22 Oct 2018  ยท  134Comments  ยท  Source: openjournals/joss-reviews

Submitting author: @lbenet (Luis Benet)
Repository: https://github.com/JuliaDiff/TaylorSeries.jl
Version: v0.9.2
Editor: @jedbrown
Reviewer: @sriharikrishna, @tobydriscoll
Archive: 10.5281/zenodo.2628898

Status

status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/8ca98df42c598c151f917b6e883be8b3"><img src="http://joss.theoj.org/papers/8ca98df42c598c151f917b6e883be8b3/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/8ca98df42c598c151f917b6e883be8b3/status.svg)](http://joss.theoj.org/papers/8ca98df42c598c151f917b6e883be8b3)

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) 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

@sriharikrishna & @tobydriscoll, 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.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @jedbrown know.

โœจ Please try and complete your review in the next two weeks โœจ

Review checklist for @sriharikrishna

Conflict of interest

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] Version: v0.9.2
  • [x] Authorship: Has the submitting author (@lbenet) 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?
  • [x] 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

  • [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)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

Software paper

  • [x] Authors: Does the paper.md 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)?

Review checklist for @tobydriscoll

Conflict of interest

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] Version: v0.9.2
  • [x] Authorship: Has the submitting author (@lbenet) 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?
  • [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?
  • [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)?
  • [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
  • [x] 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

Software paper

  • [x] Authors: Does the paper.md 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)?
accepted published recommend-accept review

Most helpful comment

It's excellent software, well documented and with all the required elements in place. I have just minor (very picky) observations:

  1. There should be some pointers to other automatic differentiation software for Julia. It's not hard to find, but links are appreciated.
  2. The display of exponents is partially obscured for me in Juno. It's find at a command shell.
  3. Maybe reconsider exporting the names "gradient" and "hessian", as those are not uncommon from other packages too. But I recognize that namespace is becoming a thorny Julia problem.
  4. The function names "integrate" and "derivative" are verb and noun. I would have preferred they both be one or the other. (I did say I was being picky here.)

All 134 comments

@sriharikrishna @tobydriscoll :wave: Welcome! The comments from whedon above outline the review process. I'll be watching this thread if you have any questions. Thanks!

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @sriharikrishna, it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) 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/openjournals/joss-reviews:

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
Attempting PDF compilation. Reticulating splines etc...

@sriharikrishna @tobydriscoll :wave: I just wanted to check in to see if you need anything in order to complete your review soon.

Thirty-hour days? ๐Ÿ˜

I should be on track to do it in the upcoming week.


Tobin Driscoll
Professor of Mathematical Sciences
Director of Undergraduate Studies
Director, Center for Applications of Mathematics in Medicine
University of Delaware, Newark, DE 19716
Twitter: @tobydriscoll
Site: http://www.tobydriscoll.net
On Nov 23, 2018, 6:01 PM -0500, Jed Brown notifications@github.com, wrote:

@sriharikrishna @tobydriscoll ๐Ÿ‘‹ I just wanted to check in to see if you need anything in order to complete your review soon.
โ€”
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

It's excellent software, well documented and with all the required elements in place. I have just minor (very picky) observations:

  1. There should be some pointers to other automatic differentiation software for Julia. It's not hard to find, but links are appreciated.
  2. The display of exponents is partially obscured for me in Juno. It's find at a command shell.
  3. Maybe reconsider exporting the names "gradient" and "hessian", as those are not uncommon from other packages too. But I recognize that namespace is becoming a thorny Julia problem.
  4. The function names "integrate" and "derivative" are verb and noun. I would have preferred they both be one or the other. (I did say I was being picky here.)

Thanks a lot @tobydriscoll for your review and observations, and sorry for my late reaction; I was traveling.

Some comments on your observations (keeping the same order):

  1. Is the suggestion to add those links in the README.md file, the documentation, or in the paper?
  2. I simply can't reproduce the problem. I think it may be an issue related to the fonts you are using.
  3. We will follow your suggestion not to export those functions; indeed, these functions are common in other packages.
  4. We are dealing with this. While we agree with your observation (and are opting for verbs) other packages use precisely those names: ForwardDiff.jl uses derivative and SymPy.jl uses integrate. Our current idea is to keep derivative, adding diff and differentiate as verbs. We will also maintain integrate, and perhaps add integ.

@tobydriscoll We have just merged https://github.com/JuliaDiff/TaylorSeries.jl/pull/188, which addresses your comments.

The concrete changes are the following:

  • We added links to other Julia packages related to automatic differentiation in the documentation.
  • We are not exporting any more gradient, jacobian, hessian, jacobian! and hessian! as suggested. A new version will be released that includes these changes.
  • We are keeping derivative but added differentiate.
  • The documentation was updated.

Regarding to point 2 in your review, we couldn't reproduce the problem. It may be related to the fonts you have, or to the specific platform; if you can provide us with more details, we could address the problem.

Thanks a lot for the review. If you have further suggestions, please let us know them.

cc @jedbrown

@jedbrown I'd like to mention that we released today v0.8.1 of our package. I hope this is ok with the current review process.

@lbenet Certainly!

@sriharikrishna How is your review going?

@jedbrown @lbenet I am very sorry for the inordinate delay.

I was able to review the software and confirm its functionality. The requirements for acceptance are met fully. I was unable, however, to check the performance claims because I do not have Mathematica readily available.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

Thanks @sriharikrishna!

@lbenet Please see my PR referenced above. Once merged, I'll re-check the paper and if all looks good, we'll be ready to archive. Thanks.

Thanks a lot @sriharikrishna.

@jedbrown There is a small typo on a keyword in https://github.com/JuliaDiff/TaylorSeries.jl/pull/192. Aside from that, LGTM.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

@lbenet Looks good to me. Please archive on Zenodo and report the DOI back here. Thanks.

@jedbrown What is the usual way of proceeding now: tagging a new minor version (0.9.0), or a patch version (0.8.2)?

Up to you. Patch/subminor version is fine.

@jedbrown Archiving done! The DOI is 10.5281/zenodo.2557004.

Looks good. In the future, I would recommend using annotated tags. I think you could change it on GitHub and the Zenodo link would still work; we don't need a new DOI even if you choose to upgrade to an annotated tag.

@whedon set 10.5281/zenodo.2557004 as archive

OK. 10.5281/zenodo.2557004 is the archive.

I will look around and try to fix it. In any case, thanks for the suggestion.

@whedon set v0.9.0 as version

OK. v0.9.0 is the version.

@openjournals/joss-eics Over to you. Thanks, @tobydriscoll and @sriharikrishna.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Hi @jedbrown, I'm actually going to send this back to you (and the authors) for now. Right now the article itself is extremely brief and doesn't contain much substance; while we don't expect a full article here, it should be 250โ€“1000 words (currently the main article is 147, which put together is a long paragraph). To me, these items need to be improved:

  • A summary describing the high-level functionality and purpose of the software for a diverse, non-specialist audience
  • A clear statement of need that illustrates the purpose of the software

Further, I think the article should be improved with more description of how the software works, some examples, and use cases.

Thanks, Kyle. That's entirely fair.

Thanks for your suggestions @kyleniemeyer; I will address them in the next few days.

@jedbrown Sorry for the long delay. I just merged https://github.com/JuliaDiff/TaylorSeries.jl/pull/197, where we addressed the suggestions by @kyleniemeyer.

We rewrote parts of the paper to clarify the purpose of the package (a polynomial manipulator implemented numerically), briefly described the way in which it is designed, emphasized our use cases, and added three examples of its usage, which are different from those already present in the documentation. The examples consist in two ways of generating the Hermite polynomials, which are directly from the recurrence relation and semi-analytically from the generating function, and a simple implementation of Picard's iteration scheme to solve initial value problems.

I hope the paper fulfill now the requirements to be accepted.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

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

% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed

0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 17 0 17 0 0 140 0 --:--:-- --:--:-- --:--:-- 140
pandoc-citeproc: reference HermitePols_wikipedia not found
Error producing PDF.
! Missing number, treated as zero.

\futurelet
l.427 than \texttt{2\^{}63-1}

Looks like we failed to compile the PDF

@lbenet Thanks. I commented on that PR. You can have whedon generate again when ready.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

One more fix... This should be ready now @jedbrown.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Looks like trouble with unicode monospace fonts. I didn't find a simple workaround -- the characters are evidently part of DejaVu Sans Mono, which I use in my editor, but I haven't gotten pandoc/xelatex to pick them up even when setting monofont: DejaVu Sans Mono in the header -- it uses the font, but doesn't find some characters that work in my editor. Perhaps @arfon has ideas.

Though the accessibility implications are tragic, embedding a PDF figure might be the simplest way to rectify this.

@jedbrown I will follow your suggestion embedding pdf (or perhaps png's ?) screen-shots of a REPL session.

With regards to the content and changes, are they ok, or do we have to do something else?

We're ultimately creating a PDF, so PDF embeddings tend to work better, including supporting selection of text from the embedding. PNG is a raster format so you lose quality and selection.

On content, would it make sense to write a brief intro to the mathematical representation and any pitfalls such as numerical stability or range of polynomial degrees where the package would be useful? This could be just one paragraph, but it's pretty abstract right now, before jumping to examples.

Thanks for the comments.

The pdf is fine; I'll check what's better, a screen-shot from a REPL session or a pdf output from a jupyter session. Another clarification with respect to this: I guess that the pdf figure refers only to the final output of the Hermite polynomials, keeping the code as a block-code, right? Or do you prefer everything (the three examples) as pdf-figures?

Regarding the content you suggest, I'll think it over and come back to you about it. Right now my concern would be to make too specialized to a broad audience...

A screenshot would typically be a raster, so not ideal. I'm not sure about Jupyter PDF output.

I think the pdf output of a Jupyter notebook is created with an intermediate LaTeX step; I'll test that and upload the figs to the repo so you can inspect them.

I have opened https://github.com/JuliaDiff/TaylorSeries.jl/pull/202, where you can find the pdf's corresponding to the output which is not properly displayed. @jedbrown Is this the solution you have in mind?

The pdf's were generated (after small modifications) from the LaTeX file obtained directly from the Jupyter notebook.

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

The GitHub PDF viewer is junk, but if you download the PDF to use a real viewer, the text is selectable (as desired). Size is wonky. What font are you using in the LaTeX? Does it work for the blocks with Unicode characters? Pandoc uses LaTeX (well, XeTeX) so the font issue may be solvable that way as well.

Thanks @jedbrown for generating the article proof; I'll experiment with the width specification forthe figures, which hopefully generates nicer size as well as figure position (e.g., after the code).

I regarding the LaTeX file used, the following packages may be relevant for the font issues: fontec, mathpazo, ucs (extended unicode support), inputenc (allow unicode in tex document).

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

@jedbrown Would the current output be acceptable? (I managed to place the figures just after the code!)

@lbenet So long as you're getting this from Jupyter, I think it would be far less distracting to have the entire code snippet in a Jupyter cell rather than line-by-line entry with julia>.

Ok, I'll change it and push another commit. The reason I kept the code-block thinking on the REPL is that it allows to copy-past (to perhaps play with it). I'm not sure if having it in the pdf allows for the same.

It does. That was part of the rationale for including those as PDF instead of PNG.

Excellent! I'm on it.

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs
. Reticulating splines etc...

@jedbrown I pushed a couple of commits to properly adjust the figures. I think they are ok, including the font size displayed. For consistency, the code corresponding to the last example is also added as figure 3.

I also have added a sentence describing the limits until which we have tested the package. Those are no hard-limits, but simply usage limits that have been sufficient for us.

I just noticed that subscripts and superscripts are gone again...

... Though this seems to occur in the generated pdf; the original pdf files display them properly.

Unicode symbols seem to be missing from the generated PDF.

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

Looks like Whedon didn't listen to your instruction to use the branch (but did heed mine). Can you check that once more to confirm whether it's a bug in Whedon?

Characters look fine to me. Some of the comments run really long. Could you shorten them or put them on their own line? There's also an almost-awkward amount of vertical whitespace compared to the main text.

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

My last attempt seems to have worked fine.

@whedon generate pdf from branch joss-pdfs

Attempting PDF compilation from custom branch joss-pdfs. Reticulating splines etc...

@jedbrown We have just addressed your comments on the content. The pdf looks ok to me. I am waiting for the green lights of travis to merge to master. Is there something else to be done?

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@jedbrown Is there something left to be done here?

@lbenet Thanks for your patience. (I was offline last week.) Please fix this citation https://github.com/JuliaDiff/TaylorSeries.jl/commit/98281c271e83193037c7805a2ccf2d6711c50b74#r33009220
There is also a comment in figure 3 that spills into the margin. Can you shorten it? We should be ready to archive once these formatting issues are fixed.

@whedon generate pdf from branch joss3

Attempting PDF compilation from custom branch joss3. Reticulating splines etc...

@jedbrown I just opened https://github.com/JuliaDiff/TaylorSeries.jl/pull/203; the citation was fixed (link to Wikipedia) and the comment of Fig 3 shortened. I also added the tex files to generate the pdf's (I cropped the pdf's manually though).

Let me know if I need to do something else before merging.

Looks good to me, thanks. Please merge and tag (annotated tag preferred) and archive on Zenodo or similar, then report the DOI back here.

Thanks a lot @jedbrown for all your work and comments.

I merged JuliaDiff/TaylorSeries.jl#203, just tagged a new minor version (0.9.2), which has been accepted in METADATA.jl (current central repo of registered packages, but not for long). (I hope I annotated it properly; sorry if not.) The DOI at zenodo (v0.9.2) is: 10.5281/zenodo.2628898.

@whedon set v0.9.2 as version

OK. v0.9.2 is the version.

@whedon set 10.5281/zenodo.2628898 as archive

OK. 10.5281/zenodo.2628898 is the archive.

@whedon accept

Attempting dry run of processing paper acceptance...

```Reference check summary:

OK DOIs

  • 10.5281/zenodo.2557003 is OK
  • 10.1137/141000671 is OK
  • 10.1007/978-3-319-29662-3 is OK
  • 10.1145/1236463.1236468 is OK
  • 10.5281/zenodo.2562364 is OK

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/607

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/607, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true

@openjournals/joss-eics Over to you, please.

@whedon accept deposit=true

Doing it live! Attempting automated processing of paper acceptance...

๐Ÿšจ๐Ÿšจ๐Ÿšจ THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! ๐Ÿšจ๐Ÿšจ๐Ÿšจ

Here's what you must now do:

  1. Check final PDF and Crossref metadata that was deposited :point_right: https://github.com/openjournals/joss-papers/pull/608
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01043
  3. If everything looks good, then close this review issue.
  4. Party like you just published a paper! ๐ŸŽ‰๐ŸŒˆ๐Ÿฆ„๐Ÿ’ƒ๐Ÿ‘ป๐Ÿค˜

    Any issues? notify your editorial technical team...

Congrats @lbenet on your article's publication in JOSS!

Thanks to @sriharikrishna and @tobydriscoll for reviewing, and @jedbrown for editing.

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippets:

Markdown:
[![DOI](http://joss.theoj.org/papers/10.21105/joss.01043/status.svg)](https://doi.org/10.21105/joss.01043)

HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01043">
  <img src="http://joss.theoj.org/papers/10.21105/joss.01043/status.svg" alt="DOI badge" >
</a>

reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.01043/status.svg
   :target: https://doi.org/10.21105/joss.01043

This is how it will look in your documentation:

DOI

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

Was this page helpful?
0 / 5 - 0 ratings