Joss-reviews: [REVIEW]: hammurabi X: a C++ package for simulating Galactic emissions

Created on 13 Nov 2019  Β·  85Comments  Β·  Source: openjournals/joss-reviews

Submitting author: @gioacchinowang (Jiaxin Wang)
Repository: https://bitbucket.org/hammurabicode/hamx
Version: v2.3.1.1
Editor: @dfm
Reviewer: @zonca, @pqrs6
Archive: 10.5281/zenodo.3687352

Status

status

Status badge code:

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

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

@zonca & @pqrs6, 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 @dfm know.

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

Review checklist for @zonca

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 (@gioacchinowang) 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 @pqrs6

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 (@gioacchinowang) 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?
AAS accepted published recommend-accept review

Most helpful comment

@gioacchinowang: Congrats on your published paper!

@zonca, @pqrs6: Thank you so much for your constructive reviews!

All 85 comments

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

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

/app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:377:in parse': (tmp/1889/joss/paper.md): did not find expected key while parsing a block mapping at line 2 column 1 (Psych::SyntaxError) from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:377:inparse_stream'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:325:in parse' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:252:inload'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:473:in block in load_file' from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:472:inopen'
from /app/vendor/ruby-2.4.4/lib/ruby/2.4.0/psych.rb:472:in load_file' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-74bc29a6a731/lib/whedon.rb:115:inload_yaml'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-74bc29a6a731/lib/whedon.rb:85:in initialize' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-74bc29a6a731/lib/whedon/processor.rb:36:innew'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-74bc29a6a731/lib/whedon/processor.rb:36:in set_paper' from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-74bc29a6a731/bin/whedon:55:inprepare'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/command.rb:27:in run' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/invocation.rb:126:ininvoke_command'
from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor.rb:387:in dispatch' from /app/vendor/bundle/ruby/2.4.0/gems/thor-0.20.3/lib/thor/base.rb:466:instart'
from /app/vendor/bundle/ruby/2.4.0/bundler/gems/whedon-74bc29a6a731/bin/whedon:116:in <top (required)>' from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:inload'
from /app/vendor/bundle/ruby/2.4.0/bin/whedon:23:in `

'

Just a quick reminder that this has been submitted in collaboration with AAS. AAS makes a small donation to JOSS to support this process. For more information check out the following links and let me know if you have any issue with this:

@gioacchinowang: I've opened a pull request on your repo that fixes the formatting of the paper so that it compiles.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

I've done a slight modification about the JOSS author, so @whedon generate pdf

@gioacchinowang: I've opened a pull request on your repo that fixes the formatting of the paper so that it compiles.

Thanks a lot! and I (and all the co-authors) agree with the publishing terms.

@dfm dear editor, we are having a slightly different author list in this JOSS paper and the AAS paper. As I'm also asking the editorial office at the AAS, does a joint publication require the same author list on both papers?

@gioacchinowang - we don’t have a policy against this so I think it’s ok for them to be different.

@gioacchinowang - we don’t have a policy against this so I think it’s ok for them to be different.

great! thanks for the message :)

@zonca, @pqrs6: I just wanted to check in to see how things are going with this review. Let me know if you have any questions!

@dfm I am at "Installation", https://bitbucket.org/hammurabicode/hamx/issues/8/joss-install-instructions
is there an expected time-frame to have the review completed?

@zonca: whoops! Sorry - didn't realize there was a conversation going on over there (I'm used to them being on GitHub). We don't have a specific time line here, but we don't like things to go quiet for more than a week - that's why I was checking in. I'll keep an eye on the issues over there.

I'm on the install stage as well, although have a bit less experience with
C++ installations so have some difficulties. Sorry for the delay.

On Thu, Nov 28, 2019 at 7:39 AM Dan Foreman-Mackey notifications@github.com
wrote:

@zonca https://github.com/zonca: whoops! Sorry - didn't realize there
was a conversation going on over there (I'm used to them being on GitHub).
We don't have a specific time line here, but we don't like things to go
quiet for more than a week - that's why I was checking in. I'll keep an eye
on the issues over there.

β€”
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/openjournals/joss-reviews/issues/1889?email_source=notifications&email_token=ABRNSOIAUS3RCNH7FUW7YYDQV63ZRA5CNFSM4JM4KKP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFMPVUQ#issuecomment-559479506,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ABRNSOKYNJK6BJWHA2HB4XLQV63ZRANCNFSM4JM4KKPQ
.

I've been able to install and run Hammurabi X and recommend publication with a few minor issues.

  1. There is a bug in the python wrapper. The executable path should not need to be set by default, but I found that I had to call the code as follows;
    sim = hpx.Hampyx(xml_path=xmlpath, exe_path=exe_path)
    where exe_path was set to my own hamx executable.
  2. A potentially related issue related to my own inexperience with C++, I had to set the LD_LIBRARY_PATH in my jupyter notebook to run the tutorial script
    %env LD_LIBRARY_PATH=/home/dwatts/software/hamx/lib:/home/dwatts/software/cfitsio-3.47/lib, a line that wasn't necessary when running hamx from bash.
  3. I didn't see any clear instructions for people wanting to contribute. I found a small typo in the python wrapper documentation and made a commit directly in bitbucket, so I don't think this is a show-stopper.
  4. I didn't see a LICENSE file in the bitbucket repository, which should be added before publishing.
  5. It's possible I don't understand the JOSS guidelines, but it seems the references in the pdf proof don't conform with https://rmarkdown.rstudio.com/authoring_bibliographies_and_citations.html#citation_syntax.
  6. In tutorial_01.ipynb's cell [18], running the simulation step after setting a mask fails, with an error terminate called after throwing an instance of 'std::runtime_error' what(): no mask file.
    It would be good to address this.

All in all, considering that I suck at compilers, this was a pretty painless process. My only suggestion would be a python wrapper with more explanations of the details, but the fact that the tutorial notebooks reproduced figures in https://arxiv.org/abs/1907.00207 was an excellent indication that I had done what the authors had intended.

@pqrs6 The citations look fine to me - can you give a specific example of what is inconsistent? Thanks!

@dfm I think I'm confused by the last bullet point in the author checklist:

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

I guess the proper syntax link might be referring to just pandoc and not the JOSS standards, but it does specify square brackets. The citations look fine to me in the proof, just not what's specified in the bullet points.

@pqrs6: The square brackets need to be in the markdown file but those are automatically converted to the ones you see in the proof when the pdf is generated.

@pqrs6 sorry for my late reply, in terms of the LICENSE, it is the COPYING file, we can rename it if you think it is necessary.
In terms of the other issues related to the jupyter notebook, I'm working on it, hopefully I can solve them in a few days.
Previously, I run the notebooks only on the docker image, and so didn't realize there are so many issues.

@gioacchinowang I think it would be good to rename the COPYING file, just to conform with standards.

Thanks!

@pqrs6 Sure, done, and I've integrated the jupyter notebook tests (with pytest and nbval plugin) into the bitbucket pipeline as Andrea suggested, now at least everything is fine in the docker image, and the issue #1 is solved, #6 is due to my fault by using absolute file path instead of the relative one, already fixed that.

@pqrs6 btw, we have a wiki page for the python wrapper, https://bitbucket.org/hammurabicode/hamx/wiki/wrapper/hampyx.md

ok, software and docs are fine, now sent feedback on paper, we are almost done.

@dfm @zonca @pqrs6 Thank you all for reviewing and suggestions, learned a lot through this process :)

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@whedon check references

Attempting to check references...

```Reference check summary:

OK DOIs

  • 10.1088/1361-6382/aacde0 is OK
  • 10.1051/0004-6361:200810564 is OK

MISSING DOIs

  • None

INVALID DOIs

  • None
    ```

@zonca @pqrs6: Thanks for your constructive reviews!

@gioacchinowang: Two minor comments on the paper:

  • Please add the DOI for Boulanger et al: https://ui.adsabs.harvard.edu/abs/2018JCAP...08..049B/exportcitation
  • "To extract information ~on~ from observations of the Galactic magnetic fields ~from observations~, Boulanger et al. (2018) proposed ~rigorous Bayesian analysis machinery~ a Bayesian method for parametric ~as well as~ and non-parametric Galactic magnetic field inference."

After you make these changes, we will pause this review until you get the DOI for your AAS Journals publication. @arfon and @crawfordsm: let us know if there's anything else we need to do to synchronize these submissions.

@dfm Thanks! Done.

@whedon generate pdf

Attempting PDF compilation. Reticulating splines etc...

@gioacchinowang β€” do update us on the status/timeline of your AAS publication, when opportune.

@labarba The AAS publication is under the final revision, and the editor expect it to be done within a few days. I have been asked to include the JOSS reference and DOI in the resubmition. Thanks for asking!

And similarly, the JOSS paper will include the AAS reference and DOI

@gioacchinowang the DOI for the JOSS publication will be: https://doi.org/10.21105/joss.01889 Once your AAS paper has been accepted and you have a DOI, let us know, and we'll do the same. Thanks!

@dfm @danielskatz @labarba Dear all, the DOI of the ApJS article is: 10.3847/1538-4365/ab72a2

@gioacchinowang: Perfect. Can you update the aas-doi and aas-journal arguments in the paper metadata then we should be good to go. Thanks!

@whedon generate pdf

@dfm Thanks! I've updated that, please check :)

@dfm - this looks good. Just an FYI that the AAS DOI doesn't resolve yet, and likely won't for another few weeks. We can either hold off publishing this paper until then or publish now with a broken link in the JOSS PDF. I'll leave that up to you and @gioacchinowang.

I think it's probably best to wait unless @gioacchinowang would really like this published before. I'll keep checking back, but @gioacchinowang perhaps you can let us know when the AAS article is published (with the link)? The JOSS workflow gets the papers published much faster so we can get a more or less simultaneous publication.

@whedon remind @dfm in 2 weeks

Reminder set for @dfm in 2 weeks

@dfm @arfon Dear all, I'd like to wait for a few weeks, there is no hurry in this, I'll let you know when the AAS article gets published, thanks!

@dfm Dear editor, I have just received the notice from AAS that the article has just been published online, https://doi.org/10.3847/1538-4365/ab72a2, they say in 2-3 hours the link will be available.

@whedon generate pdf

@whedon check references

Reference check summary:

OK DOIs

- 10.1088/1475-7516/2018/08/049 is OK
- 10.1088/1361-6382/aacde0 is OK
- 10.1051/0004-6361:200810564 is OK

MISSING DOIs

- None

INVALID DOIs

- None

@gioacchinowang: Here are some suggested edits for clarity.

  • The first two paragraphs weren't quite clear as written. Here's a proposal for what I think captures the same information somewhat more clearly:

Realistic models of Galactic emission are critical ingredients in the study of both the multi-phase interstellar medium (ISM) and precision cosmology. Models for the polarized synchrotron emission, absorption, and Faraday rotation are required in both cases. For ISM studies, these trace the physical conditions in the Galaxy and this emission is a significant foreground signal for cosmological surveys. The fundamental physical principles of the radiative transfer processes have been well understood for around half a century [@Rybicki:1979], but simple analytic models are not sufficient to capture the local structure and non-linear phenomena revealed by the precision and range of modern measurements.

  • "To meet the growing need of numerical simulation of the Galactic emission..." -> "To meet the growing need for numerical simulations of Galactic emission..."

  • "...not matching modern coding and numerical standards required by the current scientific developments in ISM and CMB foreground modelling." -> "not up to the coding and numerical standards required for studies of the ISM and cosmic microwave background (CMB) foregrounds."

  • "...simple regular fields structure..." -> "...simple regular field structures..."

@dfm Great thanks! I've revised as suggested.

@whedon generate pdf

Great, thanks! Looks good to me. Can you now tag a final release and post the Zenodo (or similar) DOI and the version number of the software for the paper? Then we should be good to go.

@dfm Yes, the new Zenodo badge has been updated.

@whedon set 10.5281/zenodo.3687352 as archive

OK. 10.5281/zenodo.3687352 is the archive.

@whedon set v2.3.1.1 as version

OK. v2.3.1.1 is the version.

@openjournals/joss-eics this paper is ready for final processing. Thanks!

Hi folks β€” I am the Associate Editor-in-Chief on rotation this week. I'm not familiar with Bitbucket, and I can't find where the version is set to v2.3.1.1. When I look at tags, I see xv.02.02 and when I look at the Wiki, it says xv.02.03 on version history.

Can you clarify for me how the version should display in the repository?

@labarba Dear Editor-in-Chief, the zenodo can only link with github account, so I made a mirror at github and get a zenodo DOI, and so the version tag was made only in github and forgot to sync it to bitbucket, now the version issue is fixed, both in the repo and the wiki.

@gioacchinowang just FYI: to archive a repository at Zenodo, you do not need to set up the GitHub-Zenodo link. Instead, wherever the software is hosted, you just need to archive the repository into a .zip, .tar.gz, etc. file and upload this to Zenodo. In the Zenodo entry metadata, you can then insert the URL pointing to the repository.

@kyleniemeyer Cool, many thanks!

@whedon accept

Attempting dry run of processing paper acceptance...

:wave: @openjournals/joss-eics, this paper is ready to be accepted and published

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

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

Hey, @gioacchinowang β€” you will have to edit the metadata of your Zenodo archive. Note that the author list does not match the paper (Zenodo grabs author list form commits). This often needs to be manually tweaked. The title should also match the paper: "hammurabi X: a C++ package for simulating Galactic emissions."

@labarba Thanks, the Zenodo title and author list have been fixed.

@gioacchinowang I proofread the paper as well and it looks good to me.

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

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

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

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

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:

@gioacchinowang: Congrats on your published paper!

@zonca, @pqrs6: Thank you so much for your constructive reviews!

Was this page helpful?
0 / 5 - 0 ratings