openjournals / joss-reviews

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

[REVIEW]: openEyeTrack - A high speed multi-threaded eye tracker for head-fixed applications #1631

Closed whedon closed 5 years ago

whedon commented 5 years ago

Submitting author: @chand-lab (Chandramouli Chandrasekaran) Repository: https://github.com/chand-lab/openEyeTrack Version: v1.0.0 Editor: @cMadan Reviewer: @conradsnicta, @thejanzimmermann Archive: 10.5281/zenodo.3515534

Status

status

Status badge code:

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

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

@conradsnicta & @thejanzimmermann, 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 @cMadan know.

Please try and complete your review in the next two weeks

Review checklist for @conradsnicta

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @thejanzimmermann

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

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:

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:

mailchand 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:

mailchand 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:

mailchand 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:

mailchand 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:

mailchand 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:

mailchand 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:

mailchand 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:

mailchand 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:

mailchand 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:

mailchand 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:

jpaolocasas 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:

mailchand 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:

mailchand 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:

mailchand commented 5 years ago

Hello @conradsnicta and @cMadan

We have now revised the paper. I think we have caught all the issues that @conradsnicta raised and have resolved them. We apologize if we have missed something because the issue list is mixed in with all the generate pdf commands. Attached is a PDF with every issue and our response. Briefly, we have made sure:

The paper.md file has been updated and the pdf generated.

Please let me know if there is something I have missed or needs to be revised.

best,

Chand Responses.pdf

conradsnicta commented 5 years ago

@cMadan - As I don't have the necessary hardware (specific camera), I am unable to tick the "Functionality" and "Performance" boxes within the review (@thejanzimmermann did tick these boxes). Other than that it looks mostly okay.

There is a limitation/problem which must be pointed out. In README.md, it's stated that the software specs include "Ubuntu 16.04". On an Ubuntu 18.04 system, after manually installing qt5 development packages (which necessitated downgrades of several other packages), I did manage to run the supplied "softwareInstall.sh" script. Without the manual intervention the installation script appeared to break.

At this point Ubuntu 16.04 is pretty old. According to the Ubuntu release cycle, 16.04 ceased to obtain hardware updates in late 2018, meaning that it's not guaranteed not install on new machines. Maintenance updates (eg. security) for 16.04 will cease in early 2021.

Given that there is already apparent installation issues on Ubuntu 18.04 and that 16.04 is not really a viable option for new machines, the software presented in this paper is subject to bit rot. In other words, its longevity is at risk without continual maintenance. There is material risk that this software will not install as is on Ubuntu 20.04.

mailchand commented 5 years ago

@conradsnicta - Great re: Functionality and Performance.

Re: 16.04 vs. 18.04 -- Thanks for raising this issue. We have an 18.04 test suite as well and now re test the software on 18.04 and see if I can mitigate the issue you have raised. We will attempt to provide a revised install script for 18.04. Your point about ensuring 18.04 and 20.04 compatibility is well taken. Here is our plan:

1) Attempt to fix softwareInstall.sh to ensure 18.04 compatibility 2) If this fails miserably, note the limitation/problem associated with it and our plans for continued development to ensure forward compatibility.

best,

Chand

mailchand commented 5 years ago

Hi @conradsnicta and @cMadan

I performed some additional installations In response to the concern @conradsnicta raised. We recognize the importance of the issue. I followed the steps we outlined in Readme.md to install openEyeTrack on an 18.04.3 ubuntu install and a 19.04 install. Both of these were done on a Zotac EN51050 machine without any existing software in them and I performed clean installs. My approach is typical of experimental rigs where the computers are often dedicated to certain functions. I for instance do this with my neurophysiology hardware.

Under such conditions, the software cleanly installs on both Ubuntu versions and also did not need manual intervention beyond the steps previously documented in Readme.md (installing Teledyne DALSA's GigE-V API etc and its attendant dependencies). We were able to get this up and running in half an hour even when starting from a completely vanilla 18.04 or 19.04 install. To help potential users more, the DALSA dependencies are now also provided as a shell script.

Of course, to support unique development environments, we hope these users will raise issues on our github page where we can provide guidance.

As evidence, I am attaching screenshots of openEyeTrack running on both 18.04 and 19.04. Based on these installs, we are very confident that openEyeTrack will be compatible with future hardware and also with newer versions of Ubuntu and not be susceptible to the bit rot issues. For Ubuntu 18.04 and 19.04, the default Qt versions are also different and so openEyeTrack is not dependent on a particular version of Qt. We emphasize that for both we just used script which just ran the qt5-default install line in the script which seems to work for both Ubuntu versions. We cannot speak to Ubuntu 20.04 because it is non existent at the moment. I have now updated the Readme.md to say that you can use it on both 18.04 and 19.04.

@conradsnicta: If you are willing to provide what steps you used when the install broke for you, we would like to include it in the Readme.md to say what to do if the install breaks on the qt install line. We also wish to add that in as a warning of potential issues that could arise to help users of this software.

Screenshot from 2019-10-07 18-23-12 Screenshot from 2019-10-07 16-12-59

Please let us know if there are any other issues to resolve!

best,

Chand

conradsnicta commented 5 years ago

@mailchand @cMadan I have a separate box with an Ubuntu 18.04 installation, which has proprietary AMD and NVIDIA drivers and repos enabled (for GPU related work via OpenCL and CUDA). Other than the installation wasn't modified to my knowledge.

It's entirely possible that the proprietary drivers and/or extra repos may hinder with the installation of openEyeTrack. This in turn may indicate that a pure vanilla installation of 18.04 may work without flaws with openEyeTrack.

I can try to install openEyeTrack within a virtual machine installation of 18.04, though this may take a few days.

mailchand commented 5 years ago

Hi @conradsnicta,

Hmm, that is entirely possible. I am sure you have way more experience dealing with Linux issues given your experience with intensive development in Linux. I personally encounter issues when trying to install new software because my dependencies are not properly structured (not that you have something like that). For instance, when trying to install DeepLabCut, an open source behavioral tracking tool that uses Tensorflow and CUDA, there have been times where I struggled to get it working.

I also ran the shell script as is. It contains sudo apt-get update and sudo apt-get upgrade commands which may or may not be helpful.

It is up to you and and @cMadan to make the decision to install on a ubuntu 18.04 virtual machine for additional tests because of your role a reviewer and @cMadan being the editor. I am also willing to add a caveat in the Readme.md if it is more expedient to say that we have only tested on clean installs and that we recommend such a strategy. Whatever makes most sense to help the eventual users of the software!

Thanks for all the care and attention to this paper.

regards,

Chand

conradsnicta commented 5 years ago

@mailchand - I hereby confirm that softwareInstall.sh works on a fresh installation of Ubuntu 18.04.3. I used a virtual machine with 4 GB ram and 32 GB disk space. (Using lower amounts, 2 GB ram and 16 GB disk space, led to "internal compiler error").

@cMadan - I believe the review is done from my side. I ticked all the boxes apart from "Functionality" and "Performance". As per https://github.com/openjournals/joss-reviews/issues/1631#issuecomment-538315331 I do not have the necessary hardware to confirm the details in these 2 boxes. However, @thejanzimmermann has ticked them.