Joss-reviews: [REVIEW]: Armadillo: a template-based C++ library for linear algebra

Created on 9 Jun 2016  路  15Comments  路  Source: openjournals/joss-reviews

Submitting author: @conradsnicta (Conrad Sanderson)
Repository: https://github.com/conradsnicta/armadillo/
Version: v7.200.1b
Editor: @arfon
Reviewer: @wrathematics
Archive: 10.5281/zenodo.55251

Status

status

Status badge code:

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

Reviewer questions

Conflict of interest

  • [x] As the reviewer I confirm that there are no conflicts of interest for me to review this work (such as being a major contributor to the software).

    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: Does the release version given match the GitHub release (v7.200.1b)?
  • [x] Archive: Does the software archive resolve?

    Functionality

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

  • [x] Functionality: Have the functional claims of the software been confirmed?
  • [x] Performance: Have any performance claims of the software been confirmed?

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

Paper PDF: 10.21105.joss.00026.pdf

  • [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

All 15 comments

/ cc @openjournals/joss-reviewers - would anyone be willing to review this submission?

If you would like to review this submission then please comment on this thread so that others know you're doing a review (so as not to duplicate effort). Something as simple as :hand: I am reviewing this will suffice.

Reviewer instructions

  • Please work through the checklist at the start of this issue.
  • If you need any further guidance/clarification take a look at the reviewer guidelines here http://joss.theoj.org/about#reviewer_guidelines
  • Please make a publication recommendation at the end of your review

Any questions, please ask for help by commenting on this issue! 馃殌

I am reviewing this.

Done reviewing. I have only a few minor comments.

The DOI for the software repository (incorrectly) lists the license as MPL 1.1, rather than MPL 2.0. This may seem minor, but it's worth nothing that MPL 1.1 is not an OSI approved license, though MPL 2.0 is. I actually had a mild moment of panic when I saw that, because my understanding is MPL 1.1 isn't GPL compatible, which would affect a large number of current users. But both the LICENSE.txt and README.txt files confirm it is being distributed under MPL 2.0.

The only other possible quibble is the performance checkbox (which I did check). No specific performance claims are made, but several vague claims are made to the general effect that it achieves high performance. I have used Armadillo in the past and that is my general impression, although I have not tested this extensively and no benchmarks are provided. I personally would like to see at least some simple benchmarks, for example comparing the overhead of using Armadillo to compute an SVD over calling LAPACK directly. Though that may be beyond the scope of this review, and probably shouldn't be a strict requirement.

I recommend publication, ideally after fixing the license statement in the DOI.

Thanks for the rapid turnaround @wrathematics!

I recommend publication, ideally after fixing the license statement in the DOI.

@conradsnicta - can you reply on this thread when you've had a chance to address this?

@arfon - I've fixed the license metadata on the Zenodo archive. It now states "License: Other (Open)". Zenodo currently doesn't have an option for MPL v2.0, so this is the closest choice.

Also, I've incorporated a few minor fixes to the references (paper.bib). Could you pull them in and regenerate the paper PDF?

Updated paper: 10.21105.joss.00026.pdf

@arfon - I've fixed the license metadata on the Zenodo archive. It now states "License: Other (Open)". Zenodo currently doesn't have an option for MPL v2.0, so this is the closest choice.

@conradsnicta - sounds good.

@conradsnicta - I think this is good to accept now. Are there any other changes you're planning on making?

/ cc @wrathematics

@arfon - no changes from my side.

However, there seems to be an inconsistency in the referencing style in the generated paper (page 2). Specifically, the ordering of the names doesn't appear right: the first author in each reference is listed as Lastname, Firstname, and then following authors are listed as Firstname, Lastname. Is this deliberate?

However, there seems to be an inconsistency in the referencing style in the generated paper (page 2). Specifically, the ordering of the names doesn't appear right: the first author in each reference is listed as Lastname, Firstname, and then following authors are listed as Firstname, Lastname. Is this deliberate?

Yeah, I'd spotted that too. I'm just using the default bibliographic style from LaTeX/Pandoc.

@arfon - Maybe try this in the preamble of paper.tex:
\usepackage{natbib}
\setcitestyle{authoryear,open={(},close={)}}
then use \citep{refname} inside the body for citations.
Finally, just before \end{document}:
\bibliographystyle{plainnat}
\bibliography{paper}

Thanks @conradsnicta, I tried adding most of this to the LaTeX template but it didn't seem to change the PDF output.

then use \citep{refname} inside the body for citations.

I don't think we can do this as we go from the Markdown to PDF (via LaTeX) in a single step here with the citations being handled by pandoc.

I'm afraid we've reached the limit of my pandoc-fu :-. I'd propose that we accept this as-is and file an issue on https://github.com/openjournals/whedon about this. Does that sound OK?

@arfon - sounds good. Please go ahead.

@wrathematics many thanks again for the rapid review here!

@conradsnicta - the DOI for this paper is http://dx.doi.org/10.21105/joss.00026 馃帀 馃殌 :boom:. The URL doesn't resolve just yet as the Crossref submission queue is a little backed up.

Was this page helpful?
0 / 5 - 0 ratings