researchart / rose7re20

3 stars 1 forks source link

Reviews on submission 152 #10

Open fabianodalp opened 4 years ago

fabianodalp commented 4 years ago

The assigned reviewers are going to post their reviews on this submission within this issue. The same thread will be used also to support the interaction with the authors.

Reviewers, please check STATUS.md to determine which badges the artifact is applying for. A description of the badges can be found here: https://re20.org/index.php/artifacts/. You will also receive an e-mail with further instructions shortly.

MattiaSalnitri commented 4 years ago

Dear authors, I based my revision on the definition of the badges available here: https://re20.org/index.php/artifacts/ and here: https://www.acm.org/publications/policies/artifact-review-badging.

Documentation: The artifacts are well documented, the linked Git hub project (https://github.com/riedlma/SECOS) is well documented as well.

Completeness: The provided artifacts are complete. Authors provided the software engine, results and datasets.

Exercisable: It was possible to run the software and check the functionalities described in the paper. Even if the provided data set is not in English.

Artifacts carefully documented and well-structured to the extent that reuse and repurposing is facilitated: Software code is not documented: very few comments are present in the software code. The comments are not well-structured, methods are not commented. Documentation on software code was not provided nor is present on the Git hub page linked.

The missing information highlighted in the last point actually prevents to modify and extend the software code (repurposing) that can be only used for replication or reproducibility (using the badges terminology).

The difference between reusable badge and the other ones lies on the “repurposing” part. Therefore, in my opinion, the artifacts cannot we awarded with this badge. I will be happy to change my opinion if the authors will provided a well-documented software code.

munterkalmsteiner commented 4 years ago

Dear @MattiaSalnitri, thank you for your review. Since you are referring to the SECOS repository, I would just like to clarify that I'm not the author of that code and, besides that I use it in my experiment, it is not part of the paper or artifact submission. The code relevant for the paper is here as documented in the installation instructions.

When referring to improving the documentation, could you point to code that would benefit from additional documentation? I'll happily improve the readability of the code if necessary. If there is need for documentation, IMO, the effort is better spent on improving the code.

LloydThinks commented 4 years ago

Hi Authors,

Thank you for your submission. Your artefact and accompanying documentation are well done, so thank you for your work on making your artefact easy to install and run. It is not clear, however, how to interact with your artefacts once they are running on my machine. A small example in the readme of what to click and how to interact with the functioning tool would be nice. Along with this description, it would be nice to know which parts you developed so that the user knows where to look.

The authors only applied for "Reusable," however, their submission is eligible for the badge "Available." Since "Available" is a better badge, I am going to recommend the badge of "Available."

Given the current state of the repository, I recommend "Available"

Detailed Comments

Functional

Installation Comments

Exercisable

Documentation

Available

Repository is available on Zenodo [2], which means that these artefacts can be accessed for an extended period of time. Please make sure to link to the Zenodo resource in the camera-ready version of your paper. Currently, it is set as anonymised footnote 8. It is preferred to link to Zenodo over GitHub as Zenodo will maintain your files longer, and there is no risk of changes to the files.

[1] https://github.com/munterkalmsteiner/inception/tree/CoClassRecommender/inception-coclass-linking [2] https://zenodo.org/record/3827169#.XtYEwZ4zY8O

munterkalmsteiner commented 4 years ago

Hi Authors,

Thank you for your submission. Your artefact and accompanying documentation are well done, so thank you for your work on making your artefact easy to install and run. It is not clear, however, how to interact with your artefacts once they are running on my machine. A small example in the readme of what to click and how to interact with the functioning tool would be nice. Along with this description, it would be nice to know which parts you developed so that the user knows where to look.

Dear Lloyd, thank you for your detailed review and improvement suggestions. I have pushed 2 commits (referenced further down) and answer to some of your comments below.

The authors only applied for "Reusable," however, their submission is eligible for the badge "Available." Since "Available" is a better badge, I am going to recommend the badge of "Available."

I applied only for the reusable badge since I thought the code for the recommender and the statistical analysis would qualify for that. Since the data and code is also permanently available on Zenodo, the available badge would also fit.

To be honest, I did not read the original ACM description of the badges until now, but it reads:

We recommend that three separate badges related to artifact review be associated with research articles in ACM publications: Artifacts Evaluated, Artifacts Available and Results Validated. These badges are considered independent and any one, two or all three can be applied to any given paper depending on review procedures developed by the journal or conference.

The ACM guidelines clearly state that the badges are independent (i.e. an artifact can qualify for more than one). The RE Artifact submission instructions however state that one badge should be argued for in status.md. I'd say, conflicting requirements, and let the chairs decide ;)

Given the current state of the repository, I recommend "Available"

Detailed Comments

Functional

Installation Comments

* Repository download from Zenodo was easy

* Your INSTALL.md has the section "Instrumentation" which involves unzipping `instrumentation.tar.gz`, but this is not stated explicitly. Perhaps adding more "unzip" comments would help.

Fixed in 966ce5066188622301df053f53964577831247ce

* No issues getting the application to run

Exercisable

* I was able to get the code running on my machine, with relative ease. See detailed comments above.

* Once the application was running, I was not sure what to do next. Your repository ReadMe should offer some pictures and a general guide to walk the user through the use of your application (or in this case, the use of your plugin).

Fixed in a9e719aed2cb6ace532bebee7ab948363a6f9d11

* The application crashed while I was playing around with it. However, I do recognise that this entire application is not yours, and so it's hardly likely that the crash has to do with your specific plugin. After a quick restart I was able to continue playing with the interface.

Documentation

* I reviewed the code linked here [1] as this is the relevant code for this artefact.

* In general, there is some documentation in the code

* In places where critical decisions must be made, the code appears to be documented to assist the developer in modifying the code

* In general, for more standard loop and control structures, there is no documentation. While the functions of these code chunks may be obvious to the original developer, that may not be the case for a newcomer.

  * It is notably that the functions and variable names are well-named, and these undocumented sections are rather small. For that reason, I believe it is fine to leave these as-is for the purpose of the assignment of the badge.
  * In the future, making code reusable should come with an understanding that documenting every code chunk (not every line) is essential to helping newcomers in interpreting you code.
Available

Repository is available on Zenodo [2], which means that these artefacts can be accessed for an extended period of time. Please make sure to link to the Zenodo resource in the camera-ready version of your paper. Currently, it is set as anonymised footnote 8. It is preferred to link to Zenodo over GitHub as Zenodo will maintain your files longer, and there is no risk of changes to the files.

Yes. As soon as the artifact is accepted, I'll link to the permanent artifacts in the camera-ready paper.

[1] https://github.com/munterkalmsteiner/inception/tree/CoClassRecommender/inception-coclass-linking [2] https://zenodo.org/record/3827169#.XtYEwZ4zY8O

fabianodalp commented 4 years ago

Hello @munterkalmsteiner @LloydThinks @MattiaSalnitri. Important clarification: since RE is not an ACM conference, we are NOT awarding ACM badges. Furthermore, it is not correct to say that available is better than reusable. The two are independent:

I think here we are discussing both, although indeed Michael asked only for one.

@MattiaSalnitri: does the answer by @munterkalmsteiner satisfy your doubt on the part that is not documented? And would you agree on granting the available badge, since there is a DOI?

LloydThinks commented 4 years ago

Hello @fabianodalp and Authors. I based my feedback on the badge descriptions listed here [1], and stand by my assessment. Given the note of "independent" badges, I recommend both the badge of "Reusable" and "Available".

My apologies if my initial assessment confused concepts known from previous years.

[1] https://re20.org/index.php/artifacts/

MattiaSalnitri commented 4 years ago

Dear all, thanks for clarifying my doubts. I checked the code here [1]:

For this reasons I agree with @fabianodalp to grant the available badge.

[1] https://github.com/munterkalmsteiner/inception/tree/CoClassRecommender/inception-coclass-linking

fabianodalp commented 4 years ago

Hi all, again. So, we certainly assign "available" here.

Regarding reusable: @LloydThinks agrees, I am not sure about @MattiaSalnitri : you both checked the same repo (inception-coclass-linking) but sort of disagree. Would a clear readme file suffice, @MattiaSalnitri ?

MattiaSalnitri commented 4 years ago

Hi Fabiano, yes, a readme file with instructions similar to [1] plus a link to the (generated) documentation will certainly help developers to understand the code and modify it (repurposing).

[1] https://github.com/researchart/rose7re20/blob/master/submissions/152-unterkalmsteiner/INSTALL.md

munterkalmsteiner commented 4 years ago

@MattiaSalnitri , fair enough. @fabianodalp I'll prepare an update until 6pm CET today (June 4).

munterkalmsteiner commented 4 years ago

@fabianodalp, I have updated the README.md and linked there to developer and setup documentation.

fabianodalp commented 4 years ago

Perfect, then I am adding also the reusable badge!