openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: CSaransh: Software Suite to Study Molecular Dynamics Simulations of Collision Cascades #1461

Closed whedon closed 5 years ago

whedon commented 5 years ago

Submitting author: @haptork (Utkarsh Bhardwaj) Repository: https://github.com/haptork/csaransh Version: v0.3.2 Editor: @katyhuff Reviewer: @jmborr, @arose Archive: 10.5281/zenodo.3434635

Status

status

Status badge code:

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

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

@jmborr & @arose, 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 @katyhuff know.

Please try and complete your review in the next two weeks

Review checklist for @jmborr

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @arose

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 5 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @jmborr, 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
whedon commented 5 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

katyhuff commented 5 years ago

@arose and @jmborr Thank you for beginning your reviews. I hope all is going well. Please don't hesitate to comment if questions arise. Some authors are also very responsive to issues in the source code repository.

danielskatz commented 5 years ago

👋 @arose and @jmborr - this is just a ping/reminder, since it's not clear what's happened on these reviews in the last couple of weeks

haptork commented 5 years ago

@katyhuff There seems to be no progress on this?

P.S. here is a talk recently given on one of the analysis that is included in csaransh. The html presentation also uses the react components from csaransh web interface.

katyhuff commented 5 years ago

I'm sorry @haptork . I am sorry to say that our reviewers seem to be quite paused and I'm looking for new ones. Summer travel schedules seem to be keeping many people from committing.

@arose I know you have begun your review. If you intend to complete it, please respond today.

labarba commented 5 years ago

@katyhuff — have you had any success looking for alternate reviewers?

jmborr commented 5 years ago

I am running the instructions in README.md in a Ubuntu 16.04 box. This I was able to do:

The duplication of this widget within section "View and Compare Cascade Info" is by design?

haptork commented 5 years ago

I am running the instructions in README.md in a Ubuntu 16.04 box. This I was able to do:

  • [x] build the C++ post-processor and run the after-build tests
  • [x] run csaransh_pp and pp.py on data/lammps to produce cascades-data.js
  • [x] run the interface as a web application with node server. Here instructions should state to npm install csaransh-server prior to npm start.

The duplication of this widget within section "View and Compare Cascade Info" is by design?

Thank you for taking this up. :)

Yes, the duplication is meant for comparison. The widget has a lock on top left. With lock open, it will show the stats for currently viewing cascade while once locked the stats get fixed on the current one selected. Thus, one cascade can be fixed on one of the widget and its stats can be compared with others as one views different cascades on the other widget with open lock. By default, the left widget is locked on some arbitrary cascade and right one would change as one selects the cascade from the main table. May be in future we can introduce some better interface to compare figures of two cascades but for now this does the job.

Please let me know of any other clarification if needed :)

arose commented 5 years ago

Installation: Does installation proceed as outlined in the documentation?

The installation went smoothly. I agree with @jmborr that npm install csaransh-server should be added to the instructions.

References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

There are a few dois missing.

arose commented 5 years ago

The IAEA challenge shows a need for visualization and analysis tools for collisional cascade molecular dynamics simulation data. Are the results of the challenge available somewhere? I could not find it on their website?

katyhuff commented 5 years ago

Thanks @arose for your review!

Thank you @jmborr for your review efforts so far. Perhaps the remainder of your review could be revisited at this point (checkboxes at the top of the page).

@haptork, can you please respond (with changes to the code, install instructions, etc.) to the requests by the reviewers so far (i.e. the comment regarding adding the npm install command to the instructions, the comment from @rose regarding the dois and the results of the iaea challenge).

haptork commented 5 years ago

Thank you @arose for reviewing.

Installation: Does installation proceed as outlined in the documentation?

The installation went smoothly. I agree with @jmborr that npm install csaransh-server should be added to the instructions.

Added to the instructions with latest commit (6114548d4da347d386cefa20da28d8db67bea651)

The IAEA challenge shows a need for visualization and analysis tools for collisional cascade molecular dynamics simulation data. Are the results of the challenge available somewhere? I could not find it on their website?

The results were e-mailed to the participants. The results and the software tools of the participants are planned to be put up on the https://cascadesdb.org/ website which is a work in progress as of now.

The database that the contest uses and the one which is shown by default in csaransh does have an indirect reference. The database page of the contest is https://www-amdis.iaea.org/CDB/challenge/, for each data point it has a webpage (https://www-amdis.iaea.org/CDB/challenge/data/001.html), which shows the DOI of the publication for the datapoint. The reference publication for the data is http://doi.org/10.1038/s41467-018-03415-5 which we have added as a reference in paper.bib.

I don’t think there is any other reference for it now. We can share the mail notifying the results. Also, MoD-PMI presentation participation mentioned in paper, now has a link in the MoD-PMI web page ( http://dpc.nifs.ac.jp/dkato/MoD-PMI2019/ ) .

Let me know if anything needs to be done from our side to get the references checked.

katyhuff commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

katyhuff commented 5 years ago

@whedon check references

whedon commented 5 years ago
Attempting to check references...
whedon commented 5 years ago

OK DOIs

- 10.1103/PhysRevB.85.024105 is OK
- https://doi.org/10.1016/S0022-3115(99)00171-3 is OK
- https://doi.org/10.1016/S0022-3115(97)00244-4 is OK
- https://doi.org/10.1016/S0022-3115(99)00170-1 is OK
- https://doi.org/10.1016/j.jnucmat.2006.02.022 is OK

MISSING DOIs

- https://doi.org/10.1016/b978-0-08-056033-5.00027-6 may be missing for title: Primary Radiation Damage Formation
- https://doi.org/10.1007/978-3-642-37456-2_14 may be missing for title: Density-Based Clustering Based on Hierarchical Density Estimates
- https://doi.org/10.1209/0295-5075/103/46003 may be missing for title: High-energy collision cascades in tungsten: Dislocation loops structure and clustering scaling laws
- https://doi.org/10.1038/s41467-018-03415-5 may be missing for title: Improving atomic displacement and replacement calculations with physically realistic damage models

INVALID DOIs

- None
haptork commented 5 years ago

@katyhuff
Added missing DOIs (commit b6d057ab6d21a7bed34e90737d10d7b98e170402) Added missing refs (commit c1300cf3547740809255e08250c6a0d9e3862436) Updated screenshot captions, removed extra line between image and caption(commit 2f6b6a8626fe128cf55a27002bc24b44d4713d1e)

katyhuff commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

katyhuff commented 5 years ago

@whedon check references

whedon commented 5 years ago
Attempting to check references...
whedon commented 5 years ago

OK DOIs

- https://doi.org/10.1016/b978-0-08-056033-5.00027-6 is OK
- 10.1103/PhysRevB.85.024105 is OK
- https://doi.org/10.1016/S0022-3115(99)00171-3 is OK
- https://doi.org/10.1016/S0022-3115(97)00244-4 is OK
- https://doi.org/10.1016/S0022-3115(99)00170-1 is OK
- https://doi.org/10.1016/j.jnucmat.2006.02.022 is OK
- https://doi.org/10.1007/978-3-642-37456-2_14 is OK
- https://doi.org/10.1209/0295-5075/103/46003 is OK
- https://doi.org/10.1038/s41467-018-03415-5 is OK

MISSING DOIs

- None

INVALID DOIs

- None
haptork commented 5 years ago

Updated a few formatting issues in the latest commit (putting captions as image placeholders).

katyhuff commented 5 years ago

@haptork Thanks for engaging actively in the review process. I've made some suggestions on the pdf itself (from before your most recent commit). Can you please consider the suggestions marked in this pdf, and let me know as soon as you feel these suggested changes have been handled?

csaransh.pdf

haptork commented 5 years ago

@katyhuff Thank you for the suggestions. I've updated the paper as per the suggestions. Please find my comments on the the suggestions here : csaransh.pdf

katyhuff commented 5 years ago

Excellent, thank you.

katyhuff commented 5 years ago

@whedon generate pdf

whedon commented 5 years ago
Attempting PDF compilation. Reticulating splines etc...
whedon commented 5 years ago

:point_right: Check article proof :page_facing_up: :point_left:

katyhuff commented 5 years ago

@whedon check references

whedon commented 5 years ago
Attempting to check references...
whedon commented 5 years ago

OK DOIs

- https://doi.org/10.1016/b978-0-08-056033-5.00027-6 is OK
- 10.1103/PhysRevB.85.024105 is OK
- https://doi.org/10.1016/S0022-3115(99)00171-3 is OK
- https://doi.org/10.1016/S0022-3115(97)00244-4 is OK
- https://doi.org/10.1016/S0022-3115(99)00170-1 is OK
- https://doi.org/10.1016/j.jnucmat.2006.02.022 is OK
- https://doi.org/10.1007/978-3-642-37456-2_14 is OK
- https://doi.org/10.1209/0295-5075/103/46003 is OK
- https://doi.org/10.1038/s41467-018-03415-5 is OK

MISSING DOIs

- None

INVALID DOIs

- None
katyhuff commented 5 years ago

Thank you @arose and @jmborr for your reviews -- we couldn't do this without you. Thank you @haptork for a strong submission and for engaging actively in the review process! I have looked over the paper, double-checked all the DOI links, and have conducted a high-level review of the code itself. Everything looks ship-shape to me. At this point, please double-check the paper yourself, review any lingering details in your code/readme/etc., and then make an archive of the reviewed software in Zenodo/figshare/other service.

Please be sure that the DOI metadata (title, authors, etc.) matches this JOSS submission. Please also let me know if the release version needs to be updated in this JOSS submission. Once that's complete, please update this thread with the DOI of the archive, and I'll move forward with accepting the submission. Until then, now is your moment for final touchups!

jmborr commented 5 years ago

@katyhuff thanks for your hard work. I feel I have to apologize here because I was very very slow with my review. I was caught at a time where my organization is under pressure to finish a ton of work before some hard deadlines. It was difficult to set aside time for the review :disappointed:

haptork commented 5 years ago

@katyhuff The zenodo DOI is: https://doi.org/10.5281/zenodo.3406703. The version number is now 0.3.2.

katyhuff commented 5 years ago

@whedon set v0.3.2 as version

whedon commented 5 years ago

OK. v0.3.2 is the version.

katyhuff commented 5 years ago

@whedon set 10.5281/zenodo.3406703 as archive

whedon commented 5 years ago

OK. 10.5281/zenodo.3406703 is the archive.

katyhuff commented 5 years ago

@openjournals/joss-eics : I believe this is ready for your review

arfon commented 5 years ago

@openjournals/joss-eics : I believe this is ready for your review

/ cc @kthyng

kthyng commented 5 years ago

Hi @haptork! A few items to wrap up your submission:

Edits for paper:

katyhuff commented 5 years ago

Wow @kthyng sorry I missed all of these things (especially the version -- it's in my checklist, so I'm not at all sure how I missed it!) Thanks for your sharp eyes!

haptork commented 5 years ago

Hi @haptork! A few items to wrap up your submission:

  • [ ] The tagged version in the github repo is v0.3.4, the release version is v0.3.3, and the version just mentioned in this conversation (and listed at the top) is v0.3.2. Can you confirm the version and make these consistent?

The zenodo archive version for the DOI I mentioned is 0.3.2 (in its metadata) which corresponds to: https://github.com/haptork/csaransh/releases/tag/v0.3.2. Please ignore the later two versions in the GitHub repo. Sorry for the confusion.

Edits for paper:

  • [ ] pg. 2 paragraph 2: should "materials for fusion, 2018" be capitalized?

Thank you. Corrected.

  • [ ] pg. 2 paragraph 2: "in arXiv preprint paper (Bhardwaj, Sand, & Warrier, 2018)" should be "in the..." and is it important to specify that it is a preprint? Was it never published in a peer-reviewed journal?

Thank you. Added the and removed preprint from the sentence. The paper is in communication in a peer reviewed journal.

  • [ ] pg.2 paragraph 3: "CascadesDB database(Hill, 2018)" needs a space

Thanks. Corrected.

  • [ ] "etc" is used repeatedly throughout the paper — please remove these except when strictly necessary.

Corrected. Thanks.

  • [ ] Figure 2: "Section to chose and compare similar clusters" should be "Section to choose and compare similar clusters"

Thanks. It had already been corrected before archiving.

  • [ ] Figure 2: "The right side shows top five clusters" should be "The right side shows the top five clusters"

Thanks. Corrected

  • [ ] Figure 3: "A high energy cascade can be composed of different sub-cascades or frangments." has a typo "fragments"

Thanks. It had already been corrected before archiving.

commit for the corrections: d9d3cb9f6e84b737ba22dfd5064d8a545cbb5cc7 zenodo archive after the corrections (if required): 10.5281/zenodo.3434635

Please let me know if anything is to be done.

kthyng commented 5 years ago

@whedon set 10.5281/zenodo.3434635 as archive