joss-reviews icon indicating copy to clipboard operation
joss-reviews copied to clipboard

[REVIEW]: ρμ: A Java library of randomization enhancements and other math utilities

Open editorialbot opened this issue 3 years ago • 7 comments
trafficstars

Submitting author: @cicirello (Vincent A Cicirello) Repository: https://github.com/cicirello/rho-mu Branch with paper.md (empty if default branch): Version: v2.3.2 Editor: @diehlpk Reviewers: @xtruan, @pritchardn Archive: Pending

Status

status

Status badge code:

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

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

@xtruan & @pritchardn, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @diehlpk 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

Checklists

📝 Checklist for @xtruan

📝 Checklist for @pritchardn

editorialbot avatar Aug 09 '22 19:08 editorialbot

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

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

@editorialbot generate pdf

editorialbot avatar Aug 09 '22 19:08 editorialbot

Software report:

github.com/AlDanial/cloc v 1.88  T=0.13 s (432.0 files/s, 116062.1 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Java                            40            922           4599           8043
Markdown                         4            127              0            450
Maven                            2             34              0            419
YAML                             8             69             39            321
TeX                              1             22              0            226
JSON                             1              0              0             24
TOML                             1              5              8              5
-------------------------------------------------------------------------------
SUM:                            57           1179           4646           9488
-------------------------------------------------------------------------------


gitinspector failed to run statistical information for the repository

editorialbot avatar Aug 09 '22 19:08 editorialbot

Wordcount for paper.md is 848

editorialbot avatar Aug 09 '22 19:08 editorialbot

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

OK DOIs

- 10.21105/joss.02448 is OK
- 10.21105/joss.00950 is OK
- 10.3390/app12115506 is OK
- 10.1145/3230636 is OK
- 10.18637/jss.v005.i08 is OK
- 10.18637/jss.v012.i07 is OK
- 10.1145/3147.3165 is OK
- 10.1093/comjnl/25.1.45 is OK
- 10.1002/9781119136378 is OK
- 10.1109/IROS.2017.8202133 is OK
- 10.1109/SSCI47803.2020.9308468 is OK
- 10.1016/j.compind.2019.08.004 is OK
- 10.1016/j.simpat.2020.102089 is OK
- 10.1080/01605682.2019.1678406 is OK
- 10.1109/ICEC.1995.489178 is OK
- 10.1109/PROC.1987.13916 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot avatar Aug 09 '22 19:08 editorialbot

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

editorialbot avatar Aug 09 '22 19:08 editorialbot

Review checklist for @xtruan

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 https://github.com/cicirello/rho-mu?
  • [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 (@cicirello) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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: 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] 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?

xtruan avatar Aug 09 '22 21:08 xtruan

Review checklist for @pritchardn

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 https://github.com/cicirello/rho-mu?
  • [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 (@cicirello) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
  • [x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines

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: 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] 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?

pritchardn avatar Aug 10 '22 01:08 pritchardn

Hi @xtruan, @pritchardn how is your review going?

diehlpk avatar Aug 23 '22 14:08 diehlpk

Hello, my review is basically complete and rather uneventful

This was a very smooth review to complete, I've run the examples, including the performance tests and made my own; everything went smoothly. The extensive README is really well put together and made reviewing very straightforward. I have some comments, but nothing prevents my recommendation to accept this.

Software Paper

Very well written, but given the performance tests which do exist in the codebase, it would be nice to see them mentioned in the paper. The methodology is itself interesting and would be useful to other library writers looking to verify their own performance claims. Given the efficiency of rho-mu's implementation (along with additional functionality of course) it is reasonable to include such a claim in the paper. Another small thing, it would be nice to have the Github links be clickable in the generated pdf, but that is a minor nitpick.

Documentation

Makes full use of standard Java documentation, the APIs look completely documented and re-iterates all required points on its homepage.

Happy to elaborate further on any points, but this looks like a really clean submission to me.

pritchardn avatar Aug 24 '22 02:08 pritchardn

@pritchardn Thank you for the feedback. I'll revise to mention the performance tests in the paper.

The GitHub links do appear to be clickable for me, although it looks like JOSS's markdown processor doesn't change their font color to make it clear that is the case. It also appears to depend on how the pdf is viewed. When using the "view article proof on github" link earlier in this thread, it seems all links are disabled (citation links and urls). But when downloading the pdf and opening locally with either Acrobat or a web browser, the links work (even though the color is same as surrounding text). One of the 2 URLs is broken in the proof because it spans multiple lines, and the line number is being interpreted as part of the URL, but that should resolve itself without the line numbers.

I'm guessing that GitHub might be disabling links in PDFs when viewed directly from GitHub. I know they sanitize SVG images to disable various things, like links, and embedded javascript, etc. So maybe they do the same thing with PDFs.

cicirello avatar Aug 24 '22 16:08 cicirello

@editorialbot generate pdf

cicirello avatar Aug 24 '22 17:08 cicirello

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

editorialbot avatar Aug 24 '22 17:08 editorialbot

@pritchardn I was able to make the URLs look clearly that they are links. URL by itself = clickable link but regular text font color; but using markdown link syntax []() fixes the color to make it clear it is a link. The "view article proof on github" still appears to disable all links, but otherwise works when viewing PDF locally.

cicirello avatar Aug 24 '22 17:08 cicirello

@editorialbot generate pdf

cicirello avatar Aug 24 '22 19:08 cicirello

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

editorialbot avatar Aug 24 '22 19:08 editorialbot

@editorialbot generate pdf

cicirello avatar Aug 24 '22 20:08 cicirello

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

editorialbot avatar Aug 24 '22 20:08 editorialbot

@pritchardn thanks again for your earlier feedback and suggestions on the paper. I revised to mention the performance tests, and also managed to get the URLs formatted as links.

cicirello avatar Aug 24 '22 20:08 cicirello

My review is complete. This package was a pleasure to review, I have no suggestions for improvement in the code, documentation, or paper. A couple of additional notes I made during the review:

  • Main library built as expected from main on Windows 10 with JDK 18 + Maven 3.8.6
  • Happy to see automated tests integrated as part of the Maven build process
  • Separate examples library built as expected from main on Windows 10 with JDK 18 + Maven 3.8.6
    • 3 Javadoc generation warnings of the form: warning: use of default constructor, which does not provide a comment (this is exceedingly minor IMO)
  • All examples run as expected
  • Was able to import package version 2.3.2 from Maven Central into my own test project
  • Documentation is fantastic and very complete
  • Paper is excellent and meets all requirements
    • Verified the new additions resulting from @pritchardn's review, all look good

xtruan avatar Aug 26 '22 02:08 xtruan

@xtruan Thank you for the feedback. I also just fixed the javadoc warnings.

cicirello avatar Aug 26 '22 14:08 cicirello

Hi @xtruan, @pritchardn thanks for finishing the review.

diehlpk avatar Aug 26 '22 15:08 diehlpk

@editorialbot generate pdf

diehlpk avatar Aug 26 '22 15:08 diehlpk

@editorialbot check references

diehlpk avatar Aug 26 '22 15:08 diehlpk

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

OK DOIs

- 10.21105/joss.02448 is OK
- 10.21105/joss.00950 is OK
- 10.3390/app12115506 is OK
- 10.1145/3230636 is OK
- 10.18637/jss.v005.i08 is OK
- 10.18637/jss.v012.i07 is OK
- 10.1145/3147.3165 is OK
- 10.1093/comjnl/25.1.45 is OK
- 10.1002/9781119136378 is OK
- 10.1109/IROS.2017.8202133 is OK
- 10.1109/SSCI47803.2020.9308468 is OK
- 10.1016/j.compind.2019.08.004 is OK
- 10.1016/j.simpat.2020.102089 is OK
- 10.1080/01605682.2019.1678406 is OK
- 10.1109/ICEC.1995.489178 is OK
- 10.1109/PROC.1987.13916 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot avatar Aug 26 '22 15:08 editorialbot

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

editorialbot avatar Aug 26 '22 15:08 editorialbot

@cicirello

Some minor comments from the editorial perspective:

  • [x] Please add the country to the affiliation
  • [x] You have some statements like used 53.3% less CPU in your paper. I think here it would be good to mention the CPU type you used for this test.

diehlpk avatar Aug 26 '22 15:08 diehlpk

@editorialbot generate pdf

cicirello avatar Aug 26 '22 16:08 cicirello

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

editorialbot avatar Aug 26 '22 16:08 editorialbot

@diehlpk I added the missing country, and I added CPU/OS details. To avoid redundancy, I put the latter at start of the short paragraph that follows the bulleted list of functionality, rather than in the two bullets containing results. Also, in the repository itself, in the directory of example programs that also contains the output from my runs, I added a simple text file with the CPU/OS details as well. This way someone who doesn't read the JOSS paper will have those details as well.

cicirello avatar Aug 26 '22 16:08 cicirello

@cicirello

Please do the following:

  • [x] Generate a new release including all changes, you made during the review.
  • [x] Upload this release to Zenodo and post the DOI here. Note that the title and authors have to match with the paper.

diehlpk avatar Aug 26 '22 18:08 diehlpk