Joss-reviews: [REVIEW]: Multiscale Solar Water Heating

[whedon]

Submitting author: @milicag (Milica Grahovac)
Repository: https://github.com/LBNL-ETA/MSWH
Version: v1.0.0
Editor: @sjpfenninger
Reviewer: @brynpickering, @nmstreethran
Archive: Pending

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

Status badge code:

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

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

@brynpickering & @nmstreethran, 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 @sjpfenninger 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 ✨

Review checklist for @brynpickering

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

• [x] I confirm that I read and will adhere to the JOSS 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 (@milicag) 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: 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 @nmstreethran

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

• [x] I confirm that I read and will adhere to the JOSS 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 (@milicag) 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

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

[whedon]

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @brynpickering, @nmstreethran it looks like you're currently assigned to review this paper :tada:.

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

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

1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/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

[whedon]

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

/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in block in find': No such file or directory - tmp/2695 (Errno::ENOENT) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:incollect!'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in find' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/lib/whedon/processor.rb:61:infind_paper_paths'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/bin/whedon:50:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/bin/whedon:119:in from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

[sjpfenninger]

@whedon generate pdf

[whedon]

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

/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in block in find': No such file or directory - tmp/2695 (Errno::ENOENT) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:incollect!'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in find' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/lib/whedon/processor.rb:61:infind_paper_paths'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/bin/whedon:50:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/bin/whedon:119:in from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

[sjpfenninger]

@milicag Can you confirm which branch of the repository the review should be looking at? I see paper.md both in master and issue5_submit_to_joss. And can you move paper.md and associated files into a subdirectory inside the repository root.

[milicag]

@sjpfenninger Please review the master branch. I have a few clarity questions. Should the sub-directory be of a certain name? Is it correct to assume that the associated files are: paper.md, paper.bib and LICENSE.md?

[milicag]

@sjpfenninger I moved the paper.md, paper.bib and LICENSE.md into a new directory called paper. Let me know if you need anything else.

[sjpfenninger]

@milicag Thanks - but LICENSE.md should always be in the root of your repository (this is independent of your JOSS submission)

[sjpfenninger]

@whedon commands

[whedon]

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Re-invite a reviewer (if they can't update checklists)
@whedon re-invite @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Set the software version at the top of the issue e.g.
@whedon set v1.0.1 as version

# Open the review issue
@whedon start review

EDITORIAL TASKS

# All commands can be run on a non-default branch, to do this pass a custom 
# branch name by following the command with `from branch custom-branch-name`.
# For example:

# Compile the paper
@whedon generate pdf

# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name

# Remind an author or reviewer to return to a review after a
# certain period of time (supported units days and weeks)
@whedon remind @reviewer in 2 weeks

# Ask Whedon to do a dry run of accepting the paper and depositing with Crossref
@whedon accept

# Ask Whedon to check the references for missing DOIs
@whedon check references

# Ask Whedon to check repository statistics for the submitted software
@whedon check repository

EiC TASKS

# Invite an editor to edit a submission (sending them an email)
@whedon invite @editor as editor

# Reject a paper
@whedon reject

# Withdraw a paper
@whedon withdraw

# Ask Whedon to actually accept the paper and deposit with Crossref
@whedon accept deposit=true

[sjpfenninger]

@whedon generate pdf from branch master

[whedon]

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

[whedon]

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

sh: 1: cd: can't cd to tmp/2695
/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in block in find': No such file or directory - tmp/2695 (Errno::ENOENT) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:incollect!'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/find.rb:43:in find' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/lib/whedon/processor.rb:61:infind_paper_paths'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/bin/whedon:50:in prepare' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:inrun'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:in invoke_command' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:indispatch'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:in start' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-364ded062842/bin/whedon:119:in from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in load' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in

'

[sjpfenninger]

@openjournals/dev Any idea what is going wrong here? I suspect it is connected to the fact that https://github.com/LBNL-ETA/MSWH/tree/issue5_submit_to_joss is set as the submission repository.

[milicag]

@sjpfenninger Thanks, I moved the license back to the root. Just some notes and maybe it helps, the paper compiled fine with whedon when it was on the root of the master. I deleted issue5_submit_to_joss branch as the PR was merged. I merged the PR since JOSS software was looking only at master when we did the first checks before the editors were assigned. Maybe whedon expects the paper to be in the root as well.

[xuanxu]

@openjournals/dev Any idea what is going wrong here? I suspect it is connected to the fact that https://github.com/LBNL-ETA/MSWH/tree/issue5_submit_to_joss is set as the submission repository.

Yeah, that was it, I've updated the URL and now it should work

[xuanxu]

@whedon generate pdf

[whedon]

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

[sjpfenninger]

@xuanxu Great, thanks! @milicag Looks like it's resolved. Might not have been the subdir, but I suggest keeping it anyway to make things neater.

[sjpfenninger]

@brynpickering , @nmstreethran I see that you still have unchecked items on your checklists -- is this because you are waiting for @milicag to address them? Could you give a brief update on what is still outstanding so that we can move this forward?

[brynpickering]

@sjpfenninger, we ended up moving discussion to the software repo, but I can give an update from my side. Overall, there's a lot of content in the submission, but I have become unsure as to whether it fits within the scope of JOSS. This concern aside, I have no new issues to raise, and see my review of this submission to be close to completion. Below are more specific comments which are being / have been addressed in individual issues.

Software

The code looks to be sound, and the tests are thorough, if not entirely comprehensive; triggering of several error/info messages is not being caught by any tests, but core mathematical functionality is well tested. The implementation could be considered a bit impenetrable for would-be contributors, since variable names aren't that human-readable.

Documentation

There is API documentation, and a Jupyter notebook as an example of a model, but otherwise the documentation is quite sparse. It probably meets the minimum JOSS requirements, but makes no attempt to go into detail on what's going on in the internals, nor to more thoroughly document the vast array of input parameters expected from the user (most of which have default values). We've gone in circles about installation instructions, but seem to have reached a point now where everyone is happy.

Paper

Following my review (https://github.com/LBNL-ETA/MSWH/issues/31), the paper has recently undergone a major revision, which I will ask Whedon to re-generate following the merging of the changes. From a quick look, I expect to only make minor comments on this update to the manuscript.

Current concern

The software is open-source and well written, but it seems to be an archive of a piece of software produced for a specific (past) project. The original focus of the paper (https://github.com/LBNL-ETA/MSWH/issues/31), and a separate conversation about contribution guidelines (https://github.com/LBNL-ETA/MSWH/issues/32), raises the question of whether there is any expectation that a community grows around developing this software further. The past project(s) focussed on California, and although it is technically feasible to model anywhere in the world, the existing parametrisation and data processing retains that geographic focus. From the PR for an update to the paper:

The water consumption profiles can be highly location specific and their development for additional climate zones would require new research efforts, although a quick approximation may be made with caution by scaling the California profiles to match the location specific estimate of the average annual water use.

I would appreciate it if this concern could be addressed by the editorial team. Perhaps it shouldn't be a concern at all.

[milicag]

@sjpfenninger @sjpfenninger @nmstreethran I'm happy to address any outstanding issues!

Regarding the current concern section, I added a sentence to the current PR branch that makes the quoted out section of the paper sound more as I had intended - the section is referring to future research. I don't think that any developer of software can provide all the possible inputs and for that reason I do give a bit of a guidance in the paper. It is essentially a tip to the user who does not reside in California. For that reason there is also an example for Bosnia. The paper by JOSS approach should be really short. The journal is, what draw me towards it, dedicated to software itself. I believe we should refocus our efforts here on the software itself.

All variables that are user facing are described in the code documentation, in each method or class. The code would be very long and hard to understand from the coding point of view if I was to use full words for each variable. I hope you understand this.

The software package is complete and that is also why I decided it is ready to be published as such. Any future research depends on the available funding and I cannot see how this relates to current state and JOSS publication. I would certainly love to have future contributions!

Please feel free to provide comment and I am here to clarify anything you might still be wondering about.

Thanks,
Milica

[sjpfenninger]

@milicag @brynpickering Thanks both for the update. I see no major issue here. @milicag You could explicitly point to the California-specific data as examples included, but they do not preclude application of the software elsewhere. @nmstreethran Could you provide an update what is outstanding from your side?

[nmstreethran]

Hi @sjpfenninger, sorry for the late response.

Based on my checklist, I only have one item remaining, which is a statement of need in the documentation. I think just a summary of the target audience would suffice as the usage of the software has already been described in the documentation here. I have checked off all other items as I believe PRs https://github.com/LBNL-ETA/MSWH/pull/33 and https://github.com/LBNL-ETA/MSWH/pull/35 will resolve them.

I opened issues https://github.com/LBNL-ETA/MSWH/issues/13, https://github.com/LBNL-ETA/MSWH/issues/14, https://github.com/LBNL-ETA/MSWH/issues/15, and https://github.com/LBNL-ETA/MSWH/issues/26 as part of my review, which have all been resolved. I can confirm that the software installation, example notebooks, web application, and tests run as expected.

Overall, I think this is a useful piece of software and I am happy to recommend to accept this submission once the outstanding PRs have been merged and the documentation is complete.

[milicag]

@sjpfenninger At @brynpickering's suggestion we are having a native speaker taking a final look at the paper.md on Monday and with that the current pull request will provide all edits the reviewers identified as needed.

I have put together this finalization issue that I intend to address as the last step, given your approval.

[milicag]

@brynpickering and @nmstreethran I just merged into the main branch the two PRs in response to the latest reviewer comment.

This PR adds the contribution guidelines and a short statement of need to the README file. After Robert, one of the co-authors provided a review I introduced his suggested edits and merged the changes into the main branch through this PR. I was able to compile the paper using your preview capabilities.

This issue summarizes the remaining steps I would like to take next.

Please let me know if there is anything additional that needs to be done.

[milicag]

@milicag @brynpickering Thanks both for the update. I see no major issue here. @milicag You could explicitly point to the California-specific data as examples included, but they do not preclude application of the software elsewhere. @nmstreethran Could you provide an update what is outstanding from your side?

I believe this is well addressed in the revised paper, however, do let me know if you find that it needs additional explanation in the paper.

[brynpickering]

@whedon generate pdf

[whedon]

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

[milicag]

@brynpickering @nmstreethran I will wait for your prompt to start addressing the remaining issue, as I'd like to tag a code release that goes with the JOSS paper, to avoid a need to re-tag.

[milicag]

Please don't hesitate to ask for edits if you find something is still needed in the paper, I am happy to address it.

[brynpickering]

@milicag thanks for your work on this, I'm happy with the result and have completed my review checklist.

A minor point (which I think you brought up in a now-closed issue/PR) is that the section of your summary relating to the components of the model could be placed in a sub-section (e.g. 'model components'), to break up the summary slightly. Similarly, a sub-section for 'future applications'/'user groups' in the Statement of Need section would work well. Not sure if this violates some strict structure that JOSS papers must adhere to (@sjpfenninger?), though.

[milicag]

@brynpickering Thank you! This is good news for me and the team. The paper is pretty short, only four pages of text, so I don't see a real reason to break it into the subsections, though I also see how that would help if someone is to read the paper really quickly. I leave it to @sjpfenninger.

[milicag]

@brynpickering, @nmstreethran - Thank you for your thorough review, it has improved many aspects of the software, especially the accessibility to users and contributors! I appreciate all your help, suggestions, and all our discussions. This was a really unique experience in these pandemic times.