researchart / rose7re20

3 stars 1 forks source link

Reviews on submission 65 #12

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.

grischal commented 4 years ago

Badges applied: available & reusable

  1. Available: The data is clearly listed on Zenodo, with a unique DOI. No problems there.

  2. Reusable: The code is made available as Jupyter notebooks, and is runnable on Google CoLab. This works smoothely, and the documentation is extensive. While individual details of BERT require some familiarity, the notebooks are so well documented that it's easy to customise them and experiment with them. I see no issues assigning the badge here, either. There's only one comment that I believe would increase the usability of the notebooks: The four Jupyter notebooks for pretrained models (in Apply_Pretrained_Model) all reference non-existing model files. My guess is that the naming was changed afterwards. This can be confusing, since these are most likely the notebooks a user would start experimenting with. I suggest adapting the names so that the notebooks run out of the box on CoLab (apart from adapting the Google Drive path).

grischal commented 4 years ago

Regarding the model names: For instance, in the second notebook (_Pretrained_NoRBERTTask1.ipynb), the model_name is _NoRBERT_NFR_e10NoSampling.pkl, while the only pkl file with a similar name would be _NoRBERT_Task1_NFR_e10NoSampling.pkl

tobhey commented 4 years ago

Dear @grischal thank you for your review. Indeed, we changed the names of the pretrained model files and missed out on updating the notebooks. Thanks for pointing that out. I've updated the names and uploaded a new version on Zenodo. Now the scripts should run out of the box, in case the files in the model zip are located in Google Drive.

Best regards, Tobias

fabianodalp commented 4 years ago

Thank you @grischal for the review, and thanks @tobhey for making already a pass. @aydemirfb can you please provide your review on this artifact?

aydemirfb commented 4 years ago

The code and the data are available, the code is clearly documented. I run the notebooks without any problems. The code is sufficiently documented. So I am ok with the badges. One minor comment is to change the semicolons in the csv files to commas, as they would be more readable in Github.

fabianodalp commented 4 years ago

Hello again everyone: if @grischal agrees with the made changes, I would propose awarding both badges. I will mark the "available" one already, and wait for Grischa's response before granting the "reusable" one.

grischal commented 4 years ago

Yes, the changes look fine to me! Please award the badges.

fabianodalp commented 4 years ago

Thank you all for the discussion! Badges awarded!