search

openjournals / joss-reviews

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

[REVIEW]: nrCascadeSim - A simulation tool for nuclear recoil cascades resulting from neutron capture #3174

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @gerudo7 (Kitty Harris) Repository: https://github.com/villano-lab/nrCascadeSim Version: 1.0.1 Editor: @mbobra Reviewer: @kelseymh, @matthewfeickert 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

Status badge code:

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

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

@kelseymh & @matthewfeickert, 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 @mbobra 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 @kelseymh

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @matthewfeickert

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

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

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
whedon commented 3 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.26 s (223.4 files/s, 41796.8 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C                               13            711            970           2536
SVG                              1              0             21           2247
C++                             11            252            251            929
Python                           6            144            236            501
C/C++ Header                    10            154            308            484
Markdown                         5             69              0            236
make                             2             22              8             69
TeX                              1              7              1             68
HTML                             1             12              1             67
Jupyter Notebook                 1              0            129             65
Bourne Shell                     2             11              3             55
YAML                             3              2              4             54
CSS                              1              6              0             31
-------------------------------------------------------------------------------
SUM:                            57           1390           1932           7342
-------------------------------------------------------------------------------

Statistical information for the repository '18788d07331ed84e3ae88cb0' was
gathered on 2021/04/15.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Anthony Villano                  2          6962              1           78.07
Kitty                            2           881           1075           21.93

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Anthony Villano            5886           84.5          0.0               26.95
Kitty                       881          100.0          1.1               25.99
whedon commented 3 years ago

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

whedon commented 3 years ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1515/iupac.70.0026 is OK
- 10.1103/PhysRevD.91.083509 is OK
- 10.1103/physrevc.43.1086 is OK
- 10.1103/PhysRevC.46.972 is OK

MISSING DOIs

- None

INVALID DOIs

- None
mbobra commented 3 years ago

👋 @kelseymh, @matthewfeickert Thank you so much for agreeing to review this submission!

Here's what you need:

Please check off the tick boxes (above, in the first comment on this thread) as you go through your review. This is an open review process, and we encourage communication between the reviewers, the submitting author, and the editor (myself). Please feel free to ask me any questions if you have them!

kelseymh commented 3 years ago

Regarding conflict of interest: @villaa and I are collaborators on the SuperCDMS experiment. We show up as authors on all SuperCDMS papers over the past several years, and as the simulations software lead in the experiment, I have worked with Anthony directly. I have no personal concerns about impartially evaluating this nrCascadeSim package (that's part of my resonsibility in the Collaboration).

I will leave the COI checkbox open for now, until I receive a followup from the editors.

matthewfeickert commented 3 years ago

@mbobra @gerudo7 The current review information has the following for version information

Repository: https://github.com/villano-lab/nrCascadeSim Version: 1.0.1

but the repository has a tag 1.0.3 that was recently released. Unless I am told otherwise, I am going to base the review only on what is available in tag v1.0.1, but am happy to change the review version if requested.

Also, as there are no version ranges given in the docs on supported versions of ROOT I am going to base my review off of the latest release of ROOT which is v6.24.00 and conduct all code assessment in the following Docker image: atlasamglab/stats-base:root6.24.00

mbobra commented 3 years ago

@matthewfeickert Thanks for looking into this. You raise a really good point.

We allow the submitting authors to make minor changes while the review is in process. It looks like the authors are following semantic versioning, so the changes in v1.0.3 and v1.0.2 should not affect your review. If, however, a reviewer raises a concern during the review process and opens an issue on the submitting author's repo, then the author must address that change and release a new version before we accept the submission for publication. In this case, a new version (1.0.4 or greater) would include the minor changes implemented in v1.0.3 and v1.0.2. Furthermore, the final publication will include your name as a reviewer and therefore indicate your stamp of approval. Let me know if you have any issues with this.

@gerudo7 Please do not make any major changes to nrCascadeSim while it is under review.

matthewfeickert commented 3 years ago

Furthermore, the final publication will include your name as a reviewer and therefore indicate your stamp of approval. Let me know if you have any issues with this

None at all. :)

nuclearGoblin commented 3 years ago 
* [x]  **Contribution and authorship:** Has the submitting author (@gerudo7) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

  * There is only 1 commit under the username @gerudo7, but it appears that a large number of commits have been contributed by a user of the same first name.

Just wanted to cofiirm that this was me and note that I think I was able to fix this. Apparently the email in my .gitconfig file wasn't associated with my GitHub account; adding it to my account seems to have given all the pseudo-account's commits to my real one.

whedon commented 3 years ago

:wave: @kelseymh, please update us on how your review is going (this is an automated reminder).

whedon commented 3 years ago

:wave: @matthewfeickert, please update us on how your review is going (this is an automated reminder).

matthewfeickert commented 3 years ago

@matthewfeickert, please update us on how your review is going (this is an automated reminder).

I have my review finished and am trying to write it up so that it is clear and informative. My goal is to post it before EOD.

matthewfeickert commented 3 years ago

Thanks for your submission and software @gerudo7. The following is my first round review of it, which goes into detail (and so isn't just noted in the first post of this Issue) only to try to help make requested revisions more actionable and clear. Unless @mbobra has any additional comments on my review I'll proceed to open up individual GitHub issues that target the topics below in your repo to make it easier to work though them as well. Also, please feel free to ask for clarification on anything as I'd like to make sure that this information is helpful and actionable to you. :+1:

Review checklist

This review is for tag v1.0.1 of the project.

Conflict of interest

Code of Conduct

General checks

JOSS asks for a minimum of 3 months of work equivalent effort for the software to be considered. What looks like the bulk of the original code was added in 2019 with development beginning to pick up in late 2020. While

git log -- . ":(exclude)paper"

is on the short side, it seems from the paper the goal would be to continue development in the future and the statement of need seems to imply that this software could be of particular interest to the dark matter direct detection community, so it seems the work and value is there.

Functionality

Reproducible example:

$ docker run --rm -ti -v $PWD:$PWD -w $PWD atlasamglab/stats-base:root6.24.00
# git clone https://github.com/villano-lab/nrCascadeSim.git
# cd nrCascadeSim/
# git checkout v1.0.1 -b v1.0.1-branch
# gcc --version | head -n 1
gcc (Debian 8.3.0-6) 8.3.0
# root --version
ROOT Version: 6.24/00
Built for linuxx8664gcc on Apr 20 2021, 08:31:00
From tags/v6-24-00@v6-24-00
# make
g++ -fPIC -c -D__GIT_VERSION=\"v1.0.1\" -IMersenne -Iinc src//isotope_info.c `root-config --cflags --glibs` -L. -Lbin/lib
mv isotope_info.o bin/lib/
mv: cannot move 'isotope_info.o' to 'bin/lib/': Not a directory
make: *** [Makefile:28: bin/lib/isotope_info.o] Error 1

If the bin/lib directory is manually created by the user (it should not need to be) then make will succeed with a large number of warnings from the vendored distribution of the Mersenne prime algorithm.

Documentation

to simulate energy deposits due to cascading of energy levels following neutron capture.

However, there is no further documentation that is of additional detail as to the relevant physics or the areas of physics that might be interested in using the software.

The main argument (no prefix) specifies the input file. (example: levelfiles/Si28_ngam_all_cascades_rfmt_sorted.txt) Making the full example: ./realizeCascades -n 100000 -o ~/output.root levelfiles/Si28_ngam_all_cascades_rfmt_sorted.txt

which is not in a separate example section or clearly marked. This also fails to indicate that the user should be in the bin directory as the executable is not made available to the user in PATH.

When all of this is taken into account, the command

./bin/realizeCascades -o output.root levelfiles/Si28_ngam_all_cascades_rfmt_sorted.txt

prints output to stdout by default with no additional logging, which makes it quite difficult to digest the relevant information. It would be good if this could be made a non-default option.

I can see that there is a example-usecase directory in the repository that is never mentioned in the README, but there is no documentation file in the directory itself and the only form of documentation is the notebook, which is a nice visualization but not a demo. There is a Conda environment.yml, but one should also include a requirement.txt, as there is nothing specific about Conda or any Conda-forge libraries that are needed for the example --- everything can be installed from PyPI.

After creating a CPython 3.8.7 virtual environment I installed the necessary libraries and then tried to run the notebook. It fails on the first cell with

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-0e4771bdf606> in <module>
     19 file = './data/file.root'
     20
---> 21 real_Lind = np.ndarray.flatten(np.asarray(h(file)[0]))
     22 real_Sor = np.ndarray.flatten(np.asarray(h(file,model='Sorenson')[0]))
     23 small_Lind = np.ndarray.flatten(np.asarray(h(file,scalefactor=0.2)[0]))

~/Code/GitHub/JOSS-reviews/nrCascadeSim-review/nrCascadeSim/example-usecase/python/hist.py in histogramable(file, binsize, binmin, binmax, labels, model, method, resolution, val, scalefactor)
     53         x = uproot.open(file)
     54         cas = x['cascade']
---> 55         en = cas.array('E')
     56         en_dep = cas.array('delE')
     57         c_id = cas.array('cid')

AttributeError: 'Model_TTree_v20' object has no attribute 'array'

The ./realizeCascades command will run the simulation a specified number of times for a given input file. Currently, this is the only program in this package. It is possible for the user to create additional programs based on teh libraries included, and we may provide additional programs with the library in the future.

Typos in the documentation should be checked for and corrected.

There is some brief description of the custom .txt file format (the "levelfiles") but it is not well formatted for Markdown and lacks a full description of the schema and clear examples.

There are also what appear to be "TODO"s written by the authors to themselves

Check that the below is correct!

that should be removed.

This program is designed to run in a Unix-based system and has been tested on Ubuntu and MacOS.

however there are no tests in the repository (and so no automated tests either). In general, if software is described as being tested on an operating system this should mean that there are automated tests that run the test suite on a specific OS.

Software paper

nuclearGoblin commented 3 years ago

@matthewfeickert @mbobra Thank you for your comments, we have begun addressing them in a new release branch (1.0.4). Would you prefer to open issues on GitHub or should we go ahead and do so?

matthewfeickert commented 3 years ago

@matthewfeickert @mbobra Thank you for your comments, we have begun addressing them in a new release branch (1.0.4). Would you prefer to open issues on GitHub or should we go ahead and do so?

I can open GitHub Issues later, but if you want to start opening them you shouldn't wait. I would highly recommend that instead of just trying to fix everything by only committing directly to one branch that you adopt a PR based GitFlow development strategy so that there is a 1:1 relationship between a GitHub Issue and a PR that resolves that Issue. It will be much easier as you're adding tests to know if you've resolved the issue properly or not if your CI passes on the PR before you merge it.

villaa commented 3 years ago

I would highly recommend that instead of just trying to fix everything by only committing directly to one branch that you adopt a PR based GitFlow development strategy so that there is a 1:1 relationship between a GitHub Issue and a PR that resolves that Issue.

@matthewfeickert I am new to PR based GitFlow development, I would appreciate any references you have.

villaa commented 3 years ago

Regarding conflict of interest: @villaa and I are collaborators on the SuperCDMS experiment. We show up as authors on all SuperCDMS papers over the past several years, and as the simulations software lead in the experiment, I have worked with Anthony directly. I have no personal concerns about impartially evaluating this nrCascadeSim package (that's part of my resonsibility in the Collaboration).

I will leave the COI checkbox open for now, until I receive a followup from the editors.

@mbobra can you respond regarding @kelseymh 's COI? If there is a COI I would be happy to help to try to identify a new reviewer. Thank you for your time.

nuclearGoblin commented 3 years ago

(Sorry about that ^ - I forgot that it would link. I prevented the linking on all the other issues.)

I went ahead and opened issues only for things we'd already started addressing so that I could try to keep our progress organized. I'm going to get 1.0.4 released here soon so we can switch over to the PR-based strategy.

matthewfeickert commented 3 years ago

@matthewfeickert I am new to PR based GitFlow development, I would appreciate any references you have.

Sure. If you search "gitflow" you'll come up with a plethora of resources, some which will give you conflicting advice on how to go about development with regards to the need for main and develop branches vs. just using a main branch (which is more common these days in the era of abundant CI — note I am not a historian of software development best practices, so downweight that as you will). Example articles that demonstrate this conflict:

If you're not familiar with using PRs for development though I'd recommend getting comfortable with that by starting off with GitHub's intro guide and also watching Anthony Sottile's step-by-step video on how to make a PR.

Note also that GitHub allows PRs to close Issues automatically on merge with the "resolves", "closes", and "fixes" keywords (among others), so it is very easy to setup a 1:1 relationship.

I would encourage you to look around at many of the well known projects that you use and love and check our their development styles as well as looking at projects that have been published in JOSS.

As a reviewer though I am probably already overstepping my role here by telling you how I would go about doing your development (sorry, not trying to overstep my bounds here or be annoying) so I'll probably stop here. I'll open up remaining Issues if there are any left that @gerudo7 hasn't hit (looks like they did a very good job covering things in Issues :+1:) already but I don't think I should be active in giving further feedback on them while you develop and revise — I'll defer to @mbobra's guidance and advice here RE: everything I just said.

mbobra commented 3 years ago

@kelseymh If you're close collaborators, I think it violates the COI. If you appear together on papers or are equal parts of a very large collaboration, then that's okay. Does this help, or is it still a bit vague?

cc @villaa

kelseymh commented 3 years ago

@mbobra CDMS is a "large" collaboration by other science's standards, but we're fairly small in high energy physics, only about 100 people. Anthony and I have worked together as part of the overal Simulations Working Group within CDMS, but we have only directly collaborated (in the vernacular) on getting the simulation geometry correct for the detector. In particular, I wasn't involved at all on this project, and wasn't even aware that it existed in any form other than "oh, we do some reweighting of events in our simulation to get the decay channels correct."

My personal feeling is that I don't have a conflict in honestly reviewing and critiquing Anthony's work (been there, done that before :-) ). However, it's the appearance that's important, and I'm comfortable with whatever you all decide.

mbobra commented 3 years ago

@kelseymh You can proceed as a reviewer. I very much appreciate your open disclosure. This is a common situation for either large scientific collaborations or large open source scientific software packages (that is, packages with many contributors) and there is precedent for this level of collaboration between a reviewer and submitting author.

villaa commented 3 years ago

@mbobra can the editor team please comment on the Mersenne Twister license issue brought up by @matthewfeickert. I am happy to address it and we have an issue for it now but I want to clarify what is needed for publication to proceed. Ultimately I would like to move to the C++ standard library implementation but that may actually force a compiler compatibility upgrade due to C++11 standard being needed.

If the code is now included in the C++11 standard library it seems to me the license for the code must be quite permissive, and it probably was even on older versions I would imagine.

Main question: what can we include in the Mersenne folder to show the license is appropriate for our use and avoid migration to the C++ standard library form of the code for now (before publication of this article) but keep it on our issue list for upgrade after publication in the future?

matthewfeickert commented 3 years ago

If the code is now included in the C++11 standard library it seems to me the license for the code must be quite permissive, and it probably was even on older versions I would imagine.

The permissibility of a license isn't the issue, but rather that code that wasn't written by the authors is being vendored with no license. Unless code explicitly uses the Unlicense or states that it is in the public domain it needs a license for anyone to be able to use it.

This is easy to fix. If you look in the vendored code for something identifiable, like this line:

<p>On my system, a Pentium III running Linux at 500 MHz, the performance test gives the following results for generation of random integers:

a Google search shows that this was code copied from Rick Wagner's website as part of his original GIGI code. I can see from there (and poking around in the directory structure of https://faculty.washington.edu/wijsman/progdists/gigi/software/GIGI/) that it used to be licensed under LGPL, but now is under BSD.

Also, the software license was changed from the GNU Lesser General Public License to a BSD license, making commercial use of this software more convenient.

We also see it pop up under BSD on nixbit (from the same search) in agreement with Rick's website.

This text appears in the code that you vendor now

Also, the software license was changed from the GNU Lesser General Public License to a BSD license, making commercial use of this software more convenient.

but a license should be a clear file. So I'd just throw a BSD licence file in there with Rick's name on it and call it good.

Ultimately I would like to move to the C++ standard library implementation but that may actually force a compiler compatibility upgrade due to C++11 standard being needed.

If the code isn't compatible with C++11 that should be noted in the requirements.

villaa commented 3 years ago

@matthewfeickert

Also, the software license was changed from the GNU Lesser General Public License to a BSD license, making commercial use of this software more convenient.

This checks out as we see it also pop up under BSD on nixbit (from the same search).

Thank you. What I need to know from here is how to change our code. Would it be by going to one of the links you list, re-downloading the code and including a license file?

villaa commented 3 years ago

Ultimately I would like to move to the C++ standard library implementation but that may actually force a compiler compatibility upgrade due to C++11 standard being needed.

If the code isn't compatible with C++11 that should be noted in the requirements.

This was noted in the new readme file. It isn't that the code is not compatible with C++11, it is. The issue is that if I include the newer version of Mersenne Twister that works through the standard library I will be adding the requirement of C++11 as far as I know. That may seem like a trivial upgrade but even recently I've used the code on machines which still use Scientific Linux and are stuck on gcc 4.4.7.

matthewfeickert commented 3 years ago

Would it be by going to one of the links you list, re-downloading the code and including a license file?

I'd just throw a BSD license file with Rick's name on in it and the year the code was copied and call it good. You can get a copy of the BSD license template from a few places online, but easiest is Wikipedia: https://en.wikipedia.org/wiki/BSD_licenses#4-clause_license_(original_%22BSD_License%22)

nuclearGoblin commented 3 years ago

We are currently working on addressing @matthewfeickert 's comments and plan to release all of the changes together as v1.1.0. We had another question come up about one of the comments:

I can see that there is a example-usecase directory in the repository that is never mentioned in the README, but there is no documentation file in the directory itself and the only form of documentation is the notebook, which is a nice visualization but not a demo.

I am having some trouble understanding what separates this visualization from a demo. I have some ideas as to what the issue might be (below), but I was hoping I could get some guidance to see if I'm looking in the right direction, so to speak.

matthewfeickert commented 3 years ago

I am having some trouble understanding what separates this visualization from a demo. I have some ideas as to what the issue might be (below), but I was hoping I could get some guidance to see if I'm looking in the right direction, so to speak.

I'm a bit short on time this week, so I can get more detailed later, but so that you get some response sooner then later:

nuclearGoblin commented 3 years ago

Okay, I think that clears things up for me. Thank you!

matthewfeickert commented 3 years ago

@mbobra @kelseymh @gerudo7 @villaa Just coming back to this before my July gets consumed by conferences. Can we have an update on the status of everything?

It seems from glancing at the project that planned revisions (which look good in terms of scope) have slowed and at the moment we only have my review (no blame or shame meant here — life happens and this year is a very exhausting one for everyone). The review has been going on for over 3 months now and I know that my time will be exceedingly restricted in July. Does it make sense to have this particular review continue for the time being? Or should we close this review and invite a resubmission once the authors are happy with their revisions?

villaa commented 3 years ago

@mbobra @matthewfeickert I would prefer this review continue. It's true that we've slowed but we have a plan to submit the upgrades based on the feedback received so far and have not heard from the second reviewer at all.

On our side, we are willing to stick with this review even if it pushes into August. I would appreciate if @matthewfeickert is suggesting that their time would be restricted until August, or if it is well beyond that (i.e. when exactly to you plan to have time for this again).

I kind of feel that sometimes reviews go for a long-ish time and asking us to resubmit now is unfair.

Can the editor comment here, please.

mbobra commented 3 years ago

👋 @kelseymh Please let me know how your review is going. Do you need any help?

mbobra commented 3 years ago

I understand both of your concerns. On one hand, @matthewfeickert has provided an inordinate amount of constructive feedback, on his own volunteer time, for a period of months. This is more time than we at JOSS want to ask of our reviewers. On the other hand, @villaa have made progress and worked to improve the code.

So I am going to pause the review. This gives the submitting authors time to improve their code and also gives reviewers @matthewfeickert and @kelseymh a break from keeping up with this review thread.

@villaa Please message me when you feel your software is ready. At that time, I'll ask @matthewfeickert and @kelseymh to take a final look and give their final yay or nay. Then we will either accept or reject this submission. If @matthewfeickert and/or @kelseymh are not available at that time, I will assign a new reviewer (or two).

Thanks all for your hard work -- I really appreciate it.

villaa commented 3 years ago

@matthewfeickert another possibility is that we can commit to having all of your comments fully addressed by the end of July and then you can finish the review out when you're back. Does that work?

matthewfeickert commented 3 years ago

Thanks @mbobra — this all sounds reasonable to me. :+1:

another possibility is that we can commit to having all of your comments fully addressed by the end of July and then you can finish the review out when you're back. Does that work?

@villaa @gerudo7 Sure. I'm not going to ask you to commit to a timeline here as I know the summer is a very busy time for everyone. So whenever things are at a state that you're happy with on your side please just ping @mbobra and me here and we'll be quite happy to resume then. :+1: I'm now marking this Issue as "Done" in my GitHub notifications so that it gets cleared off, but it will come back up the next time there's any activity on this Issue.

I kind of feel that sometimes reviews go for a long-ish time and asking us to resubmit now is unfair.

I don't know if it is any consolation, but my suggestion was made out of a concern of fairness towards you and your time. I'm projecting my own feelings from past experiences in academic publishing here, but long running reviews that are met with further requests for revisions are their own special sort of terrible and don't motivates feelings of a shared goal between authors and reviewers. I'd like your experience with JOSS to be as positive as possible!

danielskatz commented 3 years ago

👋 @villaa - Can we get an update on your progress on this?

villaa commented 3 years ago

👋 @villaa - Can we get an update on your progress on this?

@danielskatz Hello, yes. I have another paper and proposal I'm putting in which will be both finished by next Tuesday. So, I think we can address all the issues raised in the week after that.

I don't know if our reviewers are willing to re-engage at that time.

matthewfeickert commented 3 years ago

So, I think we can address all the issues raised in the week after that.

Can you clarify this more? There has been no activity on the repository for well over a month

activity

I will be honest and say that I'm don't see how this review will be done in a reasonable time. The plan was that the authors would take the summer to revise and prepare the software for review and this did not happen. I say this with no judgement as I think it is safe to assume that everyone has had a very busy summer for a multitude of reasons — I know I have.

I don't know if our reviewers are willing to re-engage at that time.

@danielskatz I think that this review should be closed and if the authors want to submit again they are welcome to. I am the only reviewer who has participated and so another reviewer would need to be found for this to even get to an approval stage. It is not fair to anyone involved in the process to have a review that started in April be continuing at this point if it is not in the final stages.

villaa commented 3 years ago

@matthewfeickert please say what you mean, I will quote our last interaction:

T— this all sounds reasonable to me. 👍

another possibility is that we can commit to having all of your comments fully addressed by the end of July and then you can finish the review out when you're back. Does that work?

Sure. I'm not going to ask you to commit to a timeline here as I know the summer is a very busy time for everyone. So whenever things are at a state that you're happy with on your side please just ping @mbobra and me here and we'll be quite happy to resume then. 👍 I'm now marking this Issue as "Done" in my GitHub notifications so that it gets cleared off, but it will come back up the next time there's any activity on this Issue.

That implies to me that we can get back to it "whenever." What you should have said is clearly: "I will no longer work on this if you let things lay for more than a month." or something like that. I'm sorry to have caused you stress. I agree you should be removed as reviewer.

In any case, @danielskatz yes, please remove @matthewfeickert from the review and either assign two new reviewers or reject the publication until we can resubmit. Sad thing is the repo is actually quite close, but if it must be resubmitted, then that's what we'll do.

mbobra commented 3 years ago

@villaa I am the editor and responsible for removing and assigning reviewers. Resubmitting doesn't actually waste any time. If the repo is almost finished, then the new reviewers will be able to go through it quite easily and give it an overall yay or nay.

@matthewfeickert has indeed spent quite a bit of time watching this. I can't in good faith ask him to continue on his volunteer time to monitor a review for half a year. So I personally think you might want to consider re-submitting. It is a win-win all around -- you get as much time as you need to polish the repository, and when you're done the review process won't take multiple months.

villaa commented 3 years ago

@mbobra Ok, fair enough. Thanks. I will try to make the next submission more efficient--good learning experience.

danielskatz commented 3 years ago

Thanks @villaa. I'll go ahead and make this happen.

danielskatz commented 3 years ago

@whedon withdraw

whedon commented 3 years ago

Paper withdrawn.

danielskatz commented 3 years ago

Also, thanks @matthewfeickert and @kelseymh and @mbobra for your effort on this!