Closed minkull closed 4 years ago
Thanks for submitting the data supporting your paper. In the below, please find the results of my efforts in trying to follow your instructions in the install.md file:
1) The authors should consider adding the figshare link to the INSTALL.MD file. One is expected to know that s/he needs to download the package before running the scripts, but the link to the package should be in the install.md file.
2) As I executed the install.r file, it gave me an error stating that no default repository was found. I searched Google for a solution and found the following link (https://stackoverflow.com/questions/33969024/install-packages-fails-in-knitr-document-trying-to-use-cran-without-setting-a). It ended up working by adding the following lines to install.r, which the authors might consider adding to the source:
r = getOption("repos")
r["CRAN"] = "http://cran.us.r-project.org"
options(repos = r)
3) There are syntax errors in install.r: install.packages("askpass")
is missing the i for install, and the command for installing purrr2
is missing an ending-quote. The authors may not have noticed that because they probably already have the packages installed;
4) After successfully installing the long list of libraries, I was able to run the "RangeOfEmotions" scripts and found the same results as published in the paper. Nice!
5) I was unable to run the first "Correlation" script. The linearMixedModel resulted in the following error:
Loading required package: Matrix
Error: package or namespace load failed for ‘LMERConvenienceFunctions’ in loadNamespace(i, c(lib.loc, .libPaths()), versionCheck = vI[[i]]):
there is no package called ‘fields’
Execution halted
6) The "Correlation" script executed well and I could find several line charts produced by the script, as well as a CSV file. However, I could not find these results in the paper. Maybe the authors could add some information in the install.md file to support finding this information in the paper.
7) The training scripts are harder to execute. I tried running Emotions.R and took the error below:
Warning message:
In doTryCatch(return(expr), name, parentenv, handler) :
unable to load shared object '/Library/Frameworks/R.framework/Resources/modules//R_X11.so':
dlopen(/Library/Frameworks/R.framework/Resources/modules//R_X11.so, 6): Library not loaded: /opt/X11/lib/libSM.6.dylib
Referenced from: /Library/Frameworks/R.framework/Versions/3.6/Resources/modules/R_X11.so
Reason: image not found
Error in py_run_file_impl(file, local, convert) :
ModuleNotFoundError: No module named 'cvxopt'
Detailed traceback:
File "<string>", line 37, in <module>
Calls: source_python -> py_run_file -> py_run_file_impl
Execution halted
8) Besides that, having data from a single subject limits the real availability of research data. I recognize that privacy is important, but the authors do not show personal data and I have not found any way to link the data to an actual person. So, I do not see a reason why we could not have access to the full dataset. Can the authors argue for this limitation?
I also encountered a few problems when running the scripts. In addition to those listed above:
1) When running linearMixedModel.R, I received the error: On MacOS, rgl depends on XQuartz, which you can download from xquartz.org. After downloading XQuartz from xquartz.org and restarting, I was able to run this script. This should be listed as a prerequisite for Mac users.
2) when running Emotions.R, I obtained a different error than described above: Error in py_module_import(module, convert = convert) : ImportError: No module named scipy.signal
3) when running "nohup ./run_HoldOut.sh SAM_valence models/models_all.txt results_SAM_valence ALL valence &> log.txt &" from the Analysis folder, I obtained the error: cannot open file 'SAM_valence': No such file or directory
4) when running "nohup ./run-tuning_LOSO.sh BySubject/Training_disambiguated_valence BySubject/Testing_disambiguated_valence disambiguated_valence models/models_all.txt resulst_SAM_valence ALL valence &> log.txt &" I obtained the error: Error: package RWeka is required
Other minor issues: “The scripts in the package can be used replicate” -> “The scripts in the package can be used to replicate”
"Analysis, contains script for tuning parameters and creating models in the two settings:" This tab should be indented
@artifact-review @anonymous-002
just an aside. I assert that "available" does not require executable products, but "reusable" does )"reusable" needs running code and some DOI link (eg. from Zenodo) to artifacts stored in some permanent storage facility (GitHub)). on the otherhand, if the material is poorly organized then available is effectively "no"
as to the current status- you want to authors to do some bug fixes before either of you can move to any decision, right?
I think the available badge would be okay if the authors make no further updates.
@anonymous-002, do you agree with available as well?
I find it strange that they gave no feedback to our comments, but they have information available and somewhat organized. I am ok with the available label.
@artifacts-reviewer @anonymous-002 @timm @minkull we apologize for not providing feedback during the discussion period. Unfortunately, although we subscribed to this issue, we didn’t receive notification for this thread. We thank you for your work and feedback. We have addressed the issues reported in your comments and updated the package accordingly.
https://github.com/researchart/rose6icse/tree/master/submissions/available/ICSE2020-main-275
Authors of the paper "Recognizing Developers’ Emotions while Programming" accepted to the 2020 technical track are:
*corresponding author
Authors applying for available badge