Joss-reviews: [REVIEW]: BoltzMM: an R package for maximum pseudolikelihood estimation of fully-visible Boltzmann machines

Created on 21 Jan 2019  Β·  69Comments  Β·  Source: openjournals/joss-reviews

Submitting author: @andrewthomasjones (Andrew Jones)
Repository: https://github.com/andrewthomasjones/BoltzMM
Version: 0.1.3
Editor: @yochannah
Reviewer: @ja-thomas, @schalkdaniel
Archive: 10.5281/zenodo.2563411

Status

status

Status badge code:

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

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

@ja-thomas & @schalkdaniel, 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 @yochannah know.

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

Review checklist for @ja-thomas

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: Does the release version given match the GitHub release (0.1.3)?
  • [x] Authorship: Has the submitting author (@andrewthomasjones) 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)?

Review checklist for @schalkdaniel

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: Does the release version given match the GitHub release (0.1.3)?
  • [x] Authorship: Has the submitting author (@andrewthomasjones) 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

all done from my side :+1:

All 69 comments

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @ja-thomas, 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...

Major comments:

  • The second author of the paper "Jessica J. Bagnall" does not seem to have commits to the repository. It might be a good idea to specify the contributions of every author in the README.

  • The tests cannot be reproduced with the instructions in the README

  • 5 out of 11 tests fail when running locally, after calling devtools::document() and devtools::test(), all with XXX has changed from known value recorded in XXX. (See https://github.com/andrewthomasjones/BoltzMM/issues/3)

Minor comments:

  • Using the shortcut T for TRUE is rather bad practice in R since it can be easily overwritten as a variable. (As done in the installation instructions)
  • knn.impute produces warnings in the example
  • Installation works (with Ubuntu 18.04). Since the version is on CRAN, installation on windows should have been checked by the WINbuilder. It still might be a good idea to add continuous integration for Linux (with Travis) and Windows (with Appveyor). Especially considering the above problems with reproducing the tests
  • A reference to the paper given the theoretical framework for this method would be nice. (Or the JOSS paper, since it contains/references the required backround).
  • There is no citation information in the README. At least the same information as citation("BoltzMM") should be given in the README

Hi @ja-thomas, thank you for taking the time to review our paper! We very much appreciate your time and effort! 😊

I will reply to some of your comments now, and @andrewthomasjones will reply to the others at a later stage.

The second author of the paper "Jessica J. Bagnall" does not seem to have commits to the repository. It might be a good idea to specify the contributions of every author in the README.

Unfortunately, Jessica is not actively on GitHub (we will persuade her, yet!) and has therefore made most of her contributions via commits by @andrewthomasjones and I. She has made numerous contributions to the paper via the documentation of the package, the senate data set that she had supplied and analysed from her paper https://hal.archives-ouvertes.fr/hal-01927188v1. As well as with identifying and helping to fix various bugs that were in previous versions of the code.

We have now included an Authorship statement section in the README in order to highlight the contributions of each of the three co-authors of the package.

knn.impute produces warnings in the example

Unfortunately, this cannot be help. No matter how the data is processed or preprocessed before conducting imputation, NAs will be created since there is presence of missingness. Thus, a warning MUST be signalled at some part in time in the workflow. Since it is a little bit unsightly to have warning messages in the README, we have opted to use a suppressWarnings() command in order to make the output more pleasing to the eye.

A reference to the paper given the theoretical framework for this method would be nice. (Or the JOSS paper, since it contains/references the required backround).

This was a great idea! We have now included a Technical references section in the README to address this comment.

There is no citation information in the README. At least the same information as citation("BoltzMM") should be given in the README

Again, great idea! We have now included a Reference to package section in the README that contains exactly the output of the command citation("BoltzMM") as you have suggested.

Thanks again for your time, @ja-thomas. It is very much appreciated.

great, thanks for the quick reply and fix.

I guess the only point open from my side are the issues with the tests discussed here https://github.com/andrewthomasjones/BoltzMM/issues/3. When this is resolved everything is fine from my side.

all done from my side :+1:

Okay, looking good! @ja-thomas and @schalkdaniel thanks for reviewing. πŸ˜„

@schalkdaniel - You still have a couple of checkboxes unchecked. When you get a moment could you let us know if your concerns are resolved, or if not what needs to be done? πŸ‘

All done from my side too. Everything looks great. πŸ‘

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

  • None

MISSING DOIs

INVALID DOIs

  • None
    ```

hmm, this is strange - these DOIs all seem to resolve correctly when I click on them.

I note there is a space as the last character for some of the DOIs which maybe we should fix, but that's only present on three so presumably can't be affecting them all!

@andrewthomasjones - it might be a good idea to remove those spaces from https://github.com/andrewthomasjones/BoltzMM/blob/master/paper.bib just in case?

@arfon / @openjournals/joss-eics - any idea what has whedon grumpy - something I missed or misunderstood maybe?

Hi @yochannah, spaces have now been removed. I also clicked on all of the links and they all seem to correctly direct to the papers. So πŸ€·β€β™‚οΈ? Thank you for you editorial work! πŸ™‡

More generally though, I think the issue is these should be doi fields, not urls, i.e. https://github.com/andrewthomasjones/BoltzMM/blob/master/paper.bib#L28 should be:

doi = {https://doi.org/10.1007/978-1-4614-6868-4}

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Hi @arfon and @yochannah, I've changed everything that should be a DOI from URL to DOI. There are a couple of items left that do not have DOIs, so I left them as URL. I hope that this resolves the issue! πŸ˜„

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

MISSING DOIs

INVALID DOIs

  • None
    ```

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

MISSING DOIs

INVALID DOIs

  • None
    ```

@arfon , it doesn't look like a DOI exists anywhere on the internet for that last paper, unfortunately. 😞

@arfon , it doesn't look like a DOI exists anywhere on the internet for that last paper, unfortunately.

That's OK. These are only meant to be suggestions - it's fine to have references without DOIs if there isn't one.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@arfon thanks for clarifying what the issue was and @hiendn thank you for resolving the issues! :+1:

@hiendn @andrewthomasjones - at this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? It's probably worth giving the manuscript a final proofread if you feel you need to, as well. πŸ‘

Hi @yochannah! Thank you for handling the editorial process. the Zenodo DOI/link is http://doi.org/10.5281/zenodo.2563411. And I have given the paper a final proof read and it is all good to go! πŸ˜„

πŸ‘‹ @hiendn β€” Could you edit the title in Zenodo so it matches the title of the JOSS paper? Thanks!

@whedon set10.5281/zenodo.2563411 as archive

@whedon set 10.5281/zenodo.2563411 as archive

OK. 10.5281/zenodo.2563411 is the archive.

Hi @labarba, the Zenodo archive name has been changed to match the paper title πŸ˜„.

@whedon accept

Attempting dry run of processing paper acceptance...

```Reference check summary:

OK DOIs

MISSING DOIs

INVALID DOIs

  • None
    ```

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

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

@hiendn β€” I'm going to have a few more editorial fixes to request.

ΒΆ1
that is able densely represent >> is able _to_
when d is large and sample size n >> what about the sample size? (missing a verb and parallel construction)

In general, I find the whole first page really jargon-heavy and I wonder if you would consider editing this to be more readable. E.g., what is "latent variable construction"? Do you mean the same by "complex form" and "elaborate form" And what _do_ you mean by that? Jus that it's hard to implement? Or is it really "intractable"? Because then you talk about a "tractable simplification" ... so many big words. Are they all necessary for the JOSS audience to plough through, to finally learn what the software does on page 2?

Many of your DOIs are not resolving because you have a full URL instead of the DOI in the BibTeX field, so the link appears broken. Please check.
[EDIT] To clarify, the links come out as https://doi.org/https://doi.org/10.1016/S0364-0213(85)80012-4 which don't resolve.

Page 2:
All of the functions that have been comprehensively documented >> delete "that"
Furthermore, the analysis of data arising from… >> Unclear how this sentence is connected with the previous and the need for "Furthermore"
complimentary >> complementary (meaning: flattering versus supportive)

It's not necessary to include the GitHub repo address or the archive address in the text, because they will appear linked in the page-1 margin of the formatted paper, and will also be linked on the paper web page. Please remove.

Overall:
I suggest a pass of ruthless editing for clarity and conciseness. For example, how does one know the difference between "comprehensively documented" and "documented," between a report that "describes" versus "clearly describes"? What is "particularly computationally intensive" versus just "computationally intensive"? When is a package "sufficiently suitable" versus "suitable"?

What I'm trying to say, as an editor, is that the paper is too long and a slog to read, and I hope you can give it a bit of a clean.

Also, note the Author Guideline that says to "include in the paper some sentences that would explain the software functionality and domain of use to a non-specialist reader."

@whedon check references

Attempting to check references...

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

Hi @labarba, thank you for the very comprehensive technical editing of the paper! πŸ™‡ Your careful reading and many suggestions have been incredibly helpful in my reorganisation of the paper to be more suitable for the lay audience of the journal. Please find my responses to your comments, below. I hope that I have addressed all of your concerns . πŸ˜„

**@hiendn β€” I'm going to have a few more editorial fixes to request.

ΒΆ1
that is able densely represent >> is able _to_**

---In light of your comments below, regarding jargon, I have chosen to replace the word densely represent, with 'closely approximate', which I feel can be interpreted at face value by a lay reader, without the need for a dive into approximation theoretic nomenclature. The grammar of the sentence has also been fixed in order to comply with the new phraseology.

when d is large and sample size n >> what about the sample size? (missing a verb and parallel construction)

---Thanks for pointing out the typo, I have now change it to 'when both dimension $d$ and sample size $n$ are large.'

In general, I find the whole first page really jargon-heavy and I wonder if you would consider editing this to be more readable.

---Thank you for this comment. I suppose that I am new to writing more lay focused papers and so a lot of my technical writing habits come through. In what follows, I endeavour to choose better words that are meaningful to a lay audience, but do not detract from the technical meaning, either.

E.g., what is "latent variable construction"?

---'Latent variable construction' is now replaced by 'requirement for hidden (or unobserved) variables'.

Do you mean the same by "complex form" and "elaborate form" And what _do_ you mean by that? Jus that it's hard to implement? Or is it really "intractable"? Because then you talk about a "tractable simplification" ... so many big words.

---You are correct in inferring that all of these phrases are just my attempt to not be repetitively writing 'intractable' over and over again. Although, with your comments in mind, I have now combed the document for unnecessary instances of words that could be replaced by things with more meaning to the reader.

Are they all necessary for the JOSS audience to plough through, to finally learn what the software does on page 2?

---I have rearranged the paragraphs of the article to be more direct in presenting the core value of the software to the reader, before going on a tangent with what the model that underlies the software is, and its history.

Many of your DOIs are not resolving because you have a full URL instead of the DOI in the BibTeX field, so the link appears broken. Please check.
[EDIT] To clarify, the links come out as https://doi.org/https://doi.org/10.1016/S0364-0213(85)80012-4 which don't resolve.

---The http... bits of the string, up to the DOIs have now been removed from each of the references. Thank you for this correction. I didn't know how the system was handling the DOIs and thought that all of the OKs from the reference checks meant that I must have been doing the right thing.

Page 2:
All of the functions that have been comprehensively documented >> delete "that"

---'that' has been deleted.

Furthermore, the analysis of data arising from… >> Unclear how this sentence is connected with the previous and the need for "Furthermore"

---'Furthermore' has been removed.

complimentary >> complementary (meaning: flattering versus supportive)

---I get these two wrong all of the time. Thank you for the correction!

It's not necessary to include the GitHub repo address or the archive address in the text, because they will appear linked in the page-1 margin of the formatted paper, and will also be linked on the paper web page. Please remove.

---Github repo and archive addresses have been removed.

Overall:
I suggest a pass of ruthless editing for clarity and conciseness. For example, how does one know the difference between "comprehensively documented" and "documented," between a report that "describes" versus "clearly describes"? What is "particularly computationally intensive" versus just "computationally intensive"? When is a package "sufficiently suitable" versus "suitable"?

---You've caught me out on one of my quirks of being overly generous with adjectives and adverbs usage. The document has been combed for the over abundance of such phraseology.

What I'm trying to say, as an editor, is that the paper is too long and a slog to read, and I hope you can give it a bit of a clean.

---Point taken. I have rearranged the document so that the interesting software stuff is at the start and the more technical stuff, which I would still like to include for completeness, appears afterwards.

Also, note the Author Guideline that says to "include in the paper some sentences that would explain the software functionality and domain of use to a non-specialist reader."

---I have included the following sentence for non-specialist readers: "Simply put, the FVBM is a model that allows for the characterization of interactions between multiple binary random variables, simultaneously."

Thanks, @hiendn.

ΒΆ1
The FVBM was first in Hyvarinen (2006) >> missing verb?

@labarba haha thanks for spotting the typo; 'considered' was the missing verb. Now included. Its amazing what you end up mentally filling in when you proof read your own writing.

@whedon accept

Attempting dry run of processing paper acceptance...

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

If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/494, 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...

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

Congratulations, @hiendn, your paper is now published!

And sincere thanks to the handling editor, @yochannah, and the reviewers, @ja-thomas, and @schalkdaniel πŸ™

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

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

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

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