Joss-reviews: [REVIEW]: OwlDE: making ODEs first-class `Owl` citizens

Created on 16 Oct 2019  Β·  54Comments  Β·  Source: openjournals/joss-reviews

Submitting author: @mseri (Marcello Seri)
Repository: https://github.com/owlbarn/owl_ode/
Version: v0.2.0
Editor: @mjsottile
Reviewers: @xoolive, @rljacobson, @mjsottile
Archive: 10.5281/zenodo.3584264

Status

status

Status badge code:

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

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

@xoolive & @rljacobson & @dagit, 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 @mjsottile know.

✨ Please try and complete your review in the next two weeks ✨

Review checklist for @xoolive

Conflict of interest

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

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] Contribution and authorship: Has the submitting author (@mseri) 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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [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] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @rljacobson

Conflict of interest

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

Code of Conduct

General checks

  • [x] 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?
  • [ ] Contribution and authorship: Has the submitting author (@mseri) 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 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

Software paper

  • [ ] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [ ] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
  • [ ] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [ ] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [ ] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

Review checklist for @mjsottile

Conflict of interest

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

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] Contribution and authorship: Has the submitting author (@mseri) 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 functionality 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] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
  • [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] State of the field: Do the authors describe how this software compares to other commonly-used packages?
  • [x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
  • [x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?
accepted published recommend-accept review

Most helpful comment

@xoolive - thanks for the review!

@mseri - I agree regarding distributing OCaml packages with external dependencies. It can be challenging but your package appears to be similar to others in terms of difficulty of installation - so that is not a significant issue in my opinion. As for Windows, the use of WSL seems to be becoming more popular, so I typically recommend people go that route instead of attempting to run OCaml natively.

@rljacobson and @dagit , let me know if you need any assistance in completing the review.

All 54 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @xoolive, @rljacobson, @dagit it looks like you're currently assigned to review 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

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

@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...

Thanks @xoolive, @rljacobson, @dagit for agreeing to review. The instructions are listed above for the review process. Feel free to reach out if you have any questions or problems.

Hello @mseri

I have first remarks about the current state of the repository/project.

  • Why is the name of the paper OwlDE when the whole project name + repository + documentation says owl-ode (which is much clearer to me to understand what it is about)
  • opam install owl-ode does not work here, even after opam update. Should we set a specific repository?
  • There are many rendering issues (markdown and LaTeX rendering) on the documentation pages, both ocaml.xyz/apidoc/ and ocaml.xyz/owl_ode

Hi @xoolive, thanks for the feedback. Let me reply what I can:

  • We changed the name in view of the official presentation of the library during ICFP 2019. The name OwlDE was chosen because it was a contraction of Owl and ODE. The underlying library name was not changed to avoid backward incompatibilities for the moment. The current plan was to rename it to owlde with a tansition owl-ode package once owl reaches 1.0 (which is currently not a known deadline).

  • This is surprising. Unless it is owl failing (there is a known problem of ubuntu's openblas that requires manual compilation of it), owl-ode should compile without problems. It is tested on multiple distributions, on ubuntu with custom openblas, and on macosx before each release, and tested by the ocaml-ci each time is released on opam. If you can provide me the logs, I can see what is wrong

  • Thanks for pointing out the issues. It is a bug in the treatment of latex in the ocaml documentation generation for which we have a workaround (I forgot to apply it during the latest documentation release). Will be fixed by monday. The apidoc issue is known and is a problem of how owl deals with documentation. This is being redesigned currently, and will be automatically fixed once owl moves to the new documentation structure.

@mseri In case it helps

$ opam update
[...]
$ opam install owl-ode
[ERROR] No package named owl-ode found.
[1]    17437 exit 66    opam install owl-ode

May I suggest you check:

opam repository list

Mine is:

  10 [http]  coq-released     https://coq.github.io/opam-coq-archive/released-opam1.2
   0 [http]     default     https://opam.ocaml.org/1.2.2

Oh I see the problem. Opam has moved to opam2 since almost one year. For some reason you are using the old unmaintained repository (and potentially still opam 1). Coq is released in the new opam2 repository as well, you should consider upgrading the system.

To avoid issues you could make the installation in a docker environment to avoid touching your system. But I understand that it would be more cumbersome.

You can find the information about the move to opam2 and how to upgrade here: https://opam.ocaml.org/doc/Upgrade_guide.html

Oh I see, I have a limited use of OCaml and wasn’t aware of the move as I have been sticking with the same packages for a while. I’ll take this as an opportunity to upgrade. Please bear with me!

No problem. Please let me know if you need any help with the upgrade

Sorry for the delay in coming back to this. I upgraded opam and tried again.

It seems the installation work but not the examples nor the tests due to some linking issues.

I would advocate for more detailed installation instructions and better dependency management. You will find below all I tried before giving up.

Sorry, I am not willing to dig more alone, by myself, into these linking issues. No problem to try more according to your instructions.

I think a troubleshooting section including opam and dependencies appearing on the github or in the documentation will help me tick the box πŸ˜‰

opam switch create 4.06.0
opam install owl-ode  # failed
sudo apt install liblapacke-dev libopenblas-dev
# try again => ok
dune exec examples/van_der_pol.exe  # fail, file does not exist



md5-5bfd78448032b4e29629609645d01a69



dune exec examples/van_der_pol_odepack.exe
opam install owl-plplot  # fail
sudo apt install libplplot-dev libshp-dev
# opam ok
# dune dependency failed again
opam install odepack
sudo apt install gfortran
# opam ok



md5-5bfd78448032b4e29629609645d01a69



# dune failing with linking issue (traceback)

/home/xo/.opam/4.06.0/.opam-switch/build/owl.0.6.0/_build/default/src/owl/src/owl/lapacke/owl_lapacke_generated_stub.c:33773: undefined reference to `LAPACKE_zlagsy'
collect2: error: ld returned 1 exit status
File "caml_startup", line 1:
Error: Error during linking
[1]    29805 exit 1     dune exec examples/van_der_pol_odepack.exe



md5-5bfd78448032b4e29629609645d01a69



dune exec examples/damped.exe  # fail, file with same linking issue



md5-5bfd78448032b4e29629609645d01a69



dune exec tests/tests.exe  # fail with dependency issue
opam install alcotest
# try again and fail with same linking issue

Hi @xoolive, thanks for the update.

The linking issue is a problem of owl and how (usually ubuntu derivatives) package openblas. I have added a more explicit troubleshooting section on owl readme and will make a PR for owl's documentation website about it in the next few days. I have also updated the README of owl_ode's repository to link to it and be more explicit about the dependencies.

To make sure you installed all the dependencies, in most cases, you can make opam deal with it for you with opam depext owl-ode owl-ode-odepack owl-ode-sundials (in fact opam depext owl-ode-sundials should be enough). This is not enough in ubuntu's case, but there is no will to change it at ubuntu level, and we are not yet shipping our custom openblas in owl (as done by e.g. numpy).

The sundials bindings use sundials3, so sundials may have to be compiled manually (the low-level bindings are provided by https://github.com/inria-parkas/sundialsml, the documentation there provides the install and troubleshooting instructions and is now linked in owl-ode readme).

To install (and run the tests) you need to ask opam for the tests dependencies: opam install -t owl-ode owl-ode-odepack owl-ode-sundials. This will also run the unit test before installing the package.

Thanks again for looking into it, I was not expecting for you to dig through these issues alone. Until now all of our users were already familiar with either the general ocaml/opam dune-based workflow or owl itself: your feedback as an external user is invaluable.

OK I managed to recompile openblas and run the examples. I have ticked all the boxes, which thereby finishes the reviewing process. I wish you the best in this project. Thank you for taking the comments into account. πŸ‘

In general, I would recommend to document the installation process more thoroughly (you addressed my comments, but I would definitely not call the result a "smooth installation process"). Maybe this comment does not particularly apply to owl-ode but more to the whole owl project. You can be helpful to a handful of users, but how long are you willing to make it?

For a newcomer to the project who could say "why not switch to OCaml because this project is nice?", there would be a lot of reasons to give up (and I didn't try to install on Windows πŸ˜†)

@xoolive thank you very much for the review, the feedback and the patience. I fully agree with your comment: Installation and distribution of owl are one of the main pain points at the moment. We are currently working on revamping its documentation system and the tutorial book, also to address the installation procedure.

The current one, available here https://ocaml.xyz/chapter/index.html, is getting out of date and is hard to maintain. The new one ( in development here https://github.com/owlbarn/owl_tutorials and on the personal forks ) will run all the code snippets as unit test and will run on docker images that are built with the instructions from the new install section. Hopefully this will help further in the smoothening of the install procedure and the onboarding on new users.

We have also considered bundling openblas itself and statically link it, but it makes the compilation time much longer. Unfortunately at the moment it is not possible to distribute binary artifacts in Ocaml. There is ongoing work to change this at the level of the compiler but it’s not going to happen soon as far as I know.

On Windows most of OCaml tooling is still catching up, and the support is very poor. You can make it work with Cygwin and wsl, but this is a problem of any OCaml program. Also there there is a lot of work going on to make the compiler and the tools more windows friendly. Things are improving fast but they are far from smooth.

@xoolive - thanks for the review!

@mseri - I agree regarding distributing OCaml packages with external dependencies. It can be challenging but your package appears to be similar to others in terms of difficulty of installation - so that is not a significant issue in my opinion. As for Windows, the use of WSL seems to be becoming more popular, so I typically recommend people go that route instead of attempting to run OCaml natively.

@rljacobson and @dagit , let me know if you need any assistance in completing the review.

@rljacobson, @dagit, can you give an update on review progress? Thanks.

@Kevin-Mattheus-Moerman I'm likely going to have to recruit an additional reviewer as the existing reviewers have gone non-responsive. Any ideas before I sift through the spreadsheet again to try to recruit one? (Would you be interested?)

@mjsottile I do not have time at the moment unfortunately. Perhaps try to ping the reviewers again and try to email them (if possible) if they remain unresponsive.

@whedon remove @dagit as reviewer

OK, @dagit is no longer a reviewer

I removed the reviewer who went completely unresponsive. I left @rljacobson for now as they have at least checked a few boxes, but appear to have stopped responding.

I am reaching out to a couple other potential reviewers off of GitHub. @mseri - do you have any suggested people I should try to recruit? If I cannot find an additional reviewer, I will do the review myself when I return from travel so this submission doesn't linger longer than it needs to.

Apologies for the delays with getting reviews completed so far.

Thanks @mjsottile. I don’t really have a suggestion, but I can think at some ocaml users that are not related with the owl project if you want.

@whedon add @mjsottile as reviewer

OK, @mjsottile is now a reviewer

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

  • None

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

@mseri I have added myself as a reviewer and am working through the review process. In the meantime, I have issued a pull request to address a typo in the paper. Also, can you add DOIs for the references in the paper? It does not appear that any of the referenced papers have DOIs.

@xoolive Can you confirm that you are satisfied with the review and would recommend acceptance? Sorry for the long delay since you finished your review.

Thanks! I will update the references between today and tomorrow

I have added the only available DOI:

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@mseri I've completed reviewing the project - looks good to me. It installed fine and I was able to run the tests and work through the examples and documentation. Given that @xoolive indicated they were done with their review and satisfied, we have two reviews now with mine.

The final step in the process before I hand it off to the EIC team is to make a tagged release and archive. JOSS needs the software to be archived in an archival repository, such as Zenodo, figshare, or an institutional repository. Once this is done, that repository should provide a DOI to the archive. Please post the DOI that you receive in this thread so I can add it to the submission. For software in GitHub, the easiest path is usually to use to the Zenodo-GitHub linkage, being sure to manually change the metadata in Zenodo so that the title and authors of the archive match the those of the JOSS paper.

Once you have completed this, I'll run the final steps and hand the submission off to get it finished up.

Thank you very much for the help. I have added the JOSS paper to the current release of OwlDE, tagged it and released it. It is now published on Zenodo with DOI 10.5281/zenodo.3584264

@whedon set 10.5281/zenodo.3584264 as archive

OK. 10.5281/zenodo.3584264 is the archive.

:wave: @openjournals/joss-eics This paper has completed the review process and I believe it's ready for final processing.

@whedon accept

Attempting dry run of processing paper acceptance...

```Reference check summary:

OK DOIs

  • 10.1088/1751-8121/ab4767 is OK

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

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

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

@whedon accept deposit=true

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

🐦🐦🐦 πŸ‘‰ Tweet for this paper πŸ‘ˆ 🐦🐦🐦

🚨🚨🚨 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/1192
  2. Wait a couple of minutes to verify that the paper DOI resolves https://doi.org/10.21105/joss.01812
  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...

@xoolive, @rljacobson, @mjsottile - many thanks for your reviews here, and to @mjsottile for editing this submission ✨

@mseri - your paper is now accepted into JOSS :zap::rocket::boom:

: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](https://joss.theoj.org/papers/10.21105/joss.01812/status.svg)](https://doi.org/10.21105/joss.01812)

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

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

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:

Thank you very much!!! πŸŽ‰πŸŽ‰πŸŽ‰

Was this page helpful?
0 / 5 - 0 ratings