search

openjournals / joss-reviews

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

[REVIEW]: DARTS: The Data Analysis Remote Treatment Service #5562

Closed editorialbot closed 1 year ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@farhi<!--end-author-handle-- (Emmanuel FARHI) Repository: https://gitlab.com/soleil-data-treatment/soleil-software-projects/remote-desktop Branch with paper.md (empty if default branch): Version: v23.10.18 Editor: !--editor-->@mbobra<!--end-editor-- Reviewers: @ebknudsen, @dfeich Archive: 10.5281/zenodo.8189000

Status

status

Status badge code:

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

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

@ebknudsen & @dfeich, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

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 ✨

Checklists

📝 Checklist for @ebknudsen

📝 Checklist for @dfeich

editorialbot commented 1 year ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 year ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.02 s (648.2 files/s, 161229.6 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Perl                             2            318            522           1320
Markdown                         5            248              0            718
HTML                             1             35             80            114
Bourne Shell                     2             17             44            100
make                             3             11             17             67
TeX                              1              6              0             51
CSS                              1              8              8             47
-------------------------------------------------------------------------------
SUM:                            15            643            671           2417
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1109/JCDL.2017.7991618 is OK
- 10.3030/957189 is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1002/smll.201802291 is INVALID because of 'https://doi.org/' prefix
editorialbot commented 1 year ago

Wordcount for paper.md is 842

editorialbot commented 1 year ago

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

mbobra commented 1 year ago

@ebknudsen @dfeich Thank you so much for agreeing to review! You can find the article in the comment boxes above ⬆️ and the software repository linked in the first comment box on this issue. To generate your checklist, use the following command: 

@editorialbot generate my checklist

I think you're good to go. Again, JOSS is an open review process and we encourage communication between the reviewers, the submitting author, and the editor. And please feel free to ask me questions, I'm always around.

Can you please respond here (or give a thumbs up) so I know you're in the right place and found all the materials?

ebknudsen commented 1 year ago

Review checklist for @ebknudsen

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

dfeich commented 1 year ago

Review checklist for @dfeich

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

ebknudsen commented 1 year ago

@mbobra I have come to realize to things:

  1. I am very late in submitting my review - which unfortunately is going to get worse, since summer holidays are upon us. If still relevant I will submit my review at the very earliest opportunity.
  2. Upon closely reading the COI-policy I have a bit of a problem: I have in the past collaborated for many years with the author, on a different topic, but we have published together repeatedly and I know him very well. This collaboration ended formally Feb. 1st 2022, with me changing jobs. I'd stress that I have not had anything to do whatsoever with the topic of this article, and feel I can definitely give the article an unbiased review. Please advise me as to what I should do?
dfeich commented 1 year ago

@mbobra Dear Monica, I took up the reviewing again this week. One of my problems mayb be that doing the installation and a full test of the SW might consume significant time. I will try to get a sufficiently large Debian VM from our services and try in the next 3 days. I tried to supply answers to all questions that I can answer right now. But for functionality, one really needs to test-drive that solution somewhere, and best using several machines (e.g. for GPU use cases). I will try to get what I can do on a minimal environment using one VM main host.

The project provides an interesting solution to an existing need. I am working myself at a site with large research facilities that are offered to visiting scientists, and it is important for such sites to provide graphical remote access to well configured analysis environments that also are well connected to storage (often data sets may have sizes of many TB).

State of the field: The paper does not line out well the other existing solutions to the problem used in the community (e.g. our site and other synchrotrons are using a commercial remote access package (nomachine) to give this kind of authorized access to specific cluster resources). So I think it would profit from giving a little overview and maybe also pointing out advantages/disadvantages of the approach. It's certainly great that the proposed solution relies purely on OSS.

The project gitlab page has quite a lot of information. But maybe the paper could use some paragraph on security. A think I wonder myself about is the performance of the VNC solution, since applications e.g. needing 3D rendering can get quite cumbersome over remote connections if the transport protocol does not try to optimize the underlying rendering data transfer.

Quality of writing: The English should be improved in some places.

dfeich commented 1 year ago

@mbobra Dear Monica, I spent some hours configuring a test environment on a Vmware Ubuntu 22.04 VM, where I installed the DEB package of qemu-web-desktop which is available from the normal repos. I was not able to get the SW running within that time. E.g. the qemu machines that I add seem not to be taken by the configuration, the "qwdctl" command does not support the "status" subcommand described in on the web page. The current DEB package also contains Soleil specific URLs and addresses. I'm sure that the author could probably fix the problems quite fast, but it seems to need quite some familiarity with the product and components.

I'm very sorry, but I cannot invest any more time on this right now, and if I needed to deploy this in real life, I would probably directly ask the developer. What do you propose for such a case like this?

Best regards, Derek

farhi commented 1 year ago

Hello @mbobra Monica,

I'm inclined to answer to the above comments, but I certainly do not wish to interfere in the review process. How should we proceed from now ?

Thanks, Emmanuel.

mbobra commented 1 year ago

I'd stress that I have not had anything to do whatsoever with the topic of this article, and feel I can definitely give the article an unbiased review. Please advise me as to what I should do?

@ebknudsen Your review is totally valid and accepted here. Thanks for checking in and please feel free to proceed.

mbobra commented 1 year ago

@mbobra Dear Monica, I spent some hours configuring a test environment on a Vmware Ubuntu 22.04 VM, where I installed the DEB package of qemu-web-desktop which is available from the normal repos. I was not able to get the SW running within that time. E.g. the qemu machines that I add seem not to be taken by the configuration, the "qwdctl" command does not support the "status" subcommand described in on the web page. The current DEB package also contains Soleil specific URLs and addresses. I'm sure that the author could probably fix the problems quite fast, but it seems to need quite some familiarity with the product and components.

I'm very sorry, but I cannot invest any more time on this right now, and if I needed to deploy this in real life, I would probably directly ask the developer. What do you propose for such a case like this?

👋 @dfeich Thanks for looking into this -- I am sorry that this took so much time (that certainly wasn't the intention). Feel free to pause for now while we get clarification from the submitting author on this issue.

mbobra commented 1 year ago

I'm inclined to answer to the above comments, but I certainly do not wish to interfere in the review process. How should we proceed from now ?

@farhi No worries -- JOSS actively encourages collaborations between the submitting author and the referees, so there's no problem with speaking up here. Is there a way for the referees to proceed with their review without spending hours of time in the setup process? The review should be lightweight.

farhi commented 1 year ago

Hello all, I'll try my best to answer in a constructive way to all of you.

First of all, thanks for taking the time to look at this contribution.

@mbobra: I can clearly see that the spirit of JOSS is different from other classical journals. The interaction between the author and the reviewers is central, and the paper itself is there to present in a formalized way the open source code. So, as far as I understand, I may dynamically modify the paper text following the comments from the reviewers. Is there a way to extend the paper length over 1000 words ? Thanks Monica for keeping us on track.

@ebknudsen: thanks for your time, Erik. I looking forward to see your comments and suggestions.

@dfeich: It seems you have already spent a lot of time on this, and I'm sorry if you did not yet succeed in what you aimed for. So far, I can see that you are attempting to accomplish 3 different tasks:

  1. evaluate the paper and source code
  2. install the software
  3. customize the software installation to your own needs (which may be outside the scope of the paper evaluation)

Regarding the paper text, its length is limited to 1000 words, and this does not let much space for further digression on security. The current length is 842 words. NoMachine does not exactly serve the same purpose. Either you have a remote display on an existing server, or you use a VM which capabilities are often limited. I'm fully opened to any suggestion regarding the phrasing, and may add a few lines about security. Regarding noVNC, on our current systems, it performs well, even for e.g. paraview. Let me know your suggestions so that we can together improve the paper, keeping within the journal constraints.

Regarding the software installation for evaluation purposes, you need to grab a physical PC, and install a basic Debian/Ubuntu system. This takes about 15 minutes from a USB key. Using a VM in order to run a VM, as you have tried, seems odd to me, especially if you further wish to pass-through a GPU. Then install the qemu-web-desktop Debian package (see https://packages.debian.org/bookworm/qemu-web-desktop). The one in the worldwide Debian/Ubuntu repos dates from Feb 2023. It does not have the qwdctl status command indeed, but all is however functional. You may also use the DEB (June 2023) attached to the latest release (https://gitlab.com/soleil-data-treatment/soleil-software-projects/remote-desktop/uploads/d14b2be83ffe6c1c7f07dd0c8ccec587/qemu-web-desktop_23.06.22_amd64.deb) or use the make install procedure from the git. As you certainly know, a software is never "finished". It takes time to push a Debian package into the worldwide repos, as there are multiple and complex validation steps to go through. For this reason, the code is a little beyond the official DEB file. Then uncomment the lines in the /etc/qemu-web-desktop/machine.conf file (for instance the Slax entry) and run sudo qwdctl download. The chosen VM will come on-line in the service, and the system will be ready for use, yet without GPU support. Connect to http://localhost/qemu-web-desktop/ and enter a fake user name as no user authentication mechanism is activated by default. The GPU configuration is much more involved, and I'm ready to discuss this point further. Last, the starting configuration file indeed contains synchrotron SOLEIL settings, which are there as examples, and are not active in the default configuration. Running with this works outside SOLEIL. Leave it untouched for the evaluation.

dfeich commented 1 year ago

Hello @farhi: Thanks for the response. I was actually not aware about the 1000 word limit, sorry, so indeed, this rather prevents any elaborations regarding security, etc. But maybe it's worth stating that the SW makes efforts to accommodate different security mechanisms.

I was doing the installation on a VM since I do not have a suitable physical machine available atm. I just wanted to do a simple functional test, so no test regarding performance or even involving any kind of GPU. I understood that the reviewer guidelines required at least a basic functional test to be done, and I also was interested to see the general structure of the code and how it works.

I can have another try with the new DEB package on the VM, I really cannot spare one of our hosts atm... I think functionally-wise it should work (I was able to launch manually launch desktops within the qemu VMS inside of the VMWare VM).

Best regards, Derek

farhi commented 1 year ago

Hi @dfeich: In my view the qemu-web-desktop can not work inside a VM, as the kernel module (KVM below QEMU) can not run. The easiest is that you grab a PC/laptop from the IT service at PSI or a trash somewhere. This will certainly do the job for the test.

mbobra commented 1 year ago

Is there a way to extend the paper length over 1000 words ?

@dfeich Let's keep it at 1000 words, but please feel free to link to documentation or other sources. Not all the references need to be refereed papers.

dfeich commented 1 year ago

Hi @farhi Sorry, but I was quite busy during the last 2 weeks due to having to conduct a number of interviews for my group. But today I had some time, and actually qemu-web-desktop works on my VMware based host, qemu desktops work via vsphere graphical web consoles, and the VNC works both inside VSphere and also from my remote connections. I can look at the basic functionality, and that is all that I needed, I did not want to investigate performance or GPU.

Using /usr/lib/cgi-bin/qemu-web-desktop.pl --dir_service=html/desktop --dir_html=html --dir_snapshots=/tmp --qemu_video=std --dir_machines=/usr/share/qemu-web-desktop/html/desktop/machines/ --oneshot=1 I was able to do a first little test through the script, and that indeed gave me a nice little slax VNC session.

What I had to do to get the web form working, was to insert by hand the contents of machines.html (the option line) into /usr/share/qemu-web-desktop/html/desktop/index.html - I thought that should have been automatically integrated by qwdctl refresh. But I can see that the part work well together, again I got a nice slax VNC session in my browser.

I will try to finish this review in the next days. Sorry for the delay, but I really wanted to run the SW. BTW: I am still not using the newest version, I downloaded the one from https://packages.debian.org/bookworm/qemu-web-desktop

Derek

ebknudsen commented 1 year ago

Just to add a (somewhat unnecessary) comment on my part - I have now returned from vacation and am all set up to do tests.

On Thu, Aug 10, 2023 at 4:21 PM Derek Feichtinger @.***> wrote:

Hi @farhi https://github.com/farhi Sorry, but I was quite busy during the last 2 weeks due to having to conduct a number of interviews for my group. But today I had some time, and actually qemu-web-desktop works on my VMware based host, qemu desktops work via vsphere graphical web consoles, and the VNC works both inside VSphere and also from my remote connections. I can look at the basic functionality, and that is all that I needed, I did not want to investigate performance or GPU.

Using /usr/lib/cgi-bin/qemu-web-desktop.pl --dir_service=html/desktop --dir_html=html --dir_snapshots=/tmp --qemu_video=std --dir_machines=/usr/share/qemu-web-desktop/html/desktop/machines/ --oneshot=1 I was able to do a first little test through the script, and that indeed gave me a nice little slax VNC session.

What I had to do to get the web form working, was to insert by hand the contents of machines.html (the option line) into /usr/share/qemu-web-desktop/html/desktop/index.html - I thought that should have been automatically integrated by qwdctl refresh. But I can see that the part work well together, again I got a nice slax VNC session in my browser.

I will try to finish this review in the next days. Sorry for the delay, but I really wanted to run the SW. BTW: I am still not using the newest version, I downloaded the one from https://packages.debian.org/bookworm/qemu-web-desktop

Derek

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/5562#issuecomment-1673318861, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQ7FEWAULTSYYU6GAOZDLXUTU6XANCNFSM6AAAAAAZGWJNXU . You are receiving this because you were mentioned.Message ID: @.***>

dfeich commented 1 year ago

Hi @mbobra. I am done so far.

@farhi I still think that it would be good to have one sentence or so mentioning how such use cases are handled by others. Even though it's not functionally doing the same, solutions involving nomachine, fastx, and similar seem to be used frequently for offering remote access/analysis to large research facility compute resources. Your approach is nicely open source, it easily scales in a straightforward way, and the VMs have the beauty that copies of those can be given to end users.

Best regards, Derek

farhi commented 1 year ago

Hello @dfeich and @mbobra . I'm going to add a few sentences around existing virtualization frameworks. I still have about 150 words left in the paper text, this should do.

farhi commented 1 year ago

I have now pushed an improved version of the manuscript at:

The source manuscript and generated PDF are attached. paper.pdf paper.zip

farhi commented 1 year ago

Command for manual paper generation:

pandoc --filter pandoc-citeproc --bibliography=paper.bib --variable classoption=twocolumn --variable papersize=a4paper -s paper.md -o paper.pdf
mbobra commented 1 year ago

@dfeich Do @farhi's changes satisfy the remaining two checklist items? If so, could you please check them off and let me know if you approve this paper for publication? Thank you!

mbobra commented 1 year ago

👋 @ebknudsen Checking in with the review -- do you need any help? Please let us know how it is going.

ebknudsen commented 1 year ago

So far so good!

On Mon, Sep 18, 2023, 20:46 Monica Bobra @.***> wrote:

👋 @ebknudsen https://github.com/ebknudsen Checking in with the review -- do you need any help? Please let us know how it is going.

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/5562#issuecomment-1724184471, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGQ7FG4VDSBPEQYGQBVE5DX3CJJ3ANCNFSM6AAAAAAZGWJNXU . You are receiving this because you were mentioned.Message ID: @.***>

ebknudsen commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

ebknudsen commented 1 year ago

I have now finally gone through the process of re-reading this paper - I like it very much. I have some tiny editorial comments.

Status on reviewing the software: I've installed and been able to run a virtual machine on a typical newly installed wiped machine. That was a very smooth experience and is very much automated. Granted I took the recommended route based on a debian machine, and so did not go through the manual steps.

Remaining to be done:

mbobra commented 1 year ago

@farhi I think we're getting pretty close here! Do you think you'll be able to wrap this up in the next month or so?

farhi commented 1 year ago

Hi @mbobra, I have just pushed an update of the manuscript at:

taking into account the last comments from @ebknudsen

paper.md paper.pdf

farhi commented 1 year ago

I'm looking forward for this process to complete, within a month seems absolutely reasonable I guess.

farhi commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

dfeich commented 1 year ago

Hello @mbobra and @farhi . Sorry, I'm just on vacation, and somehow I thought I had already given my full approval. Checked off all the boxes, now. I like the paper as it is now and with the references to similar SW, and short motivation why this is a good, relatively lightweight, and free alternative. This is a nice project. Best regards to all of you.

ebknudsen commented 1 year ago

@dfeich Well - I has not been you that has held this up. I have to take full blame for being slow. Status on my part is I'm almost done with my tests that I feel I need to do. I am happy with the structure of the paper and agree with @dfeich, that this is a good and free alternative.

mbobra commented 1 year ago

@ebknudsen Do you still need some time for the tests, or do you approve the paper for publication? If it is the latter, please check off all the boxes so I can proceed with the acceptance process.

ebknudsen commented 1 year ago

@mbobra I'm alomst done. I'll get the boxes checked tomorrow night local time.

ebknudsen commented 1 year ago

Dear all, I have now spent enough of time testing things and I am very pleased with the result. DARTS is indeed easy to set up, well documented, and lightweight. It all works as "advertised" in the paper. While I find there is not a specific section in the article about contributing I find it rather self-evident from the git-lab instance. As mentioned before - I am happy with the paper as is - now I am also convinced regarding the claims about the capabilities of the software package.

Good work @farhi! And thank you for your patience

ebknudsen commented 1 year ago

Go ahead and proceed @mbobra

mbobra commented 1 year ago

Thank you both so very much for your thorough reviews, @ebknudsen and @dfeich! Your volunteer time keeps JOSS running and the JOSS team appreciates it very much ☀️

mbobra commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

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

mbobra commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1002/smll.201802291 is OK
- 10.1109/JCDL.2017.7991618 is OK
- 10.3030/957189 is OK
- 10.17199/NOBUGS2016.65 is OK

MISSING DOIs

- None

INVALID DOIs

- https://doi.org/10.1016/j.ecoinf.2016.08.003 is INVALID because of 'https://doi.org/' prefix
mbobra commented 1 year ago

@farhi Could you please fix the invalid citation listed above ⬆️ ? See this example for an example paper and bibliography.

farhi commented 1 year ago

The citation is fixed.

farhi commented 1 year ago

@editorialbot generate pdf