openjournals / joss-reviews

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

[REVIEW]: ropenblas: Download, Compile and Link OpenBLAS Library with R #2769

Closed whedon closed 3 years ago

whedon commented 4 years ago

Submitting author: @prdm0 (Pedro Rafael Marinho) Repository: https://github.com/prdm0/ropenblas Version: v0.2.9 Editor: @timtroendle Reviewers: @pratikvn, @myousefi2016 Archive: 10.5281/zenodo.4618251

:warning: JOSS reduced service mode :warning:

Due to the challenges of the COVID-19 pandemic, JOSS is currently operating in a "reduced service mode". You can read more about what that means in our blog post.

Status

status

Status badge code:

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

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

@pratikvn & @sahilseth, 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 @timtroendle 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

Review checklist for @pratikvn

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @myousefi2016

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

pratikvn commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

pratikvn commented 3 years ago

@whedon check references

whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- None

MISSING DOIs

- None

INVALID DOIs

- None
pratikvn commented 3 years ago

@timtroendle , Everything looks good to me modulo some final paper changes which I have provided on https://github.com/prdm0/ropenblas/issues/21 .

After these changes have been implemented, I can close that issue and we can proceed here.

prdm0 commented 3 years ago

@pratikvn, thanks for the suggestions. I will proceed with the improvements and will soon generate a new PDF.

prdm0 commented 3 years ago

Dear @pratikvn and @timtroendle, I made all the requested changes. @pratikvn I made the corrections to the text you requested, I eliminated unnecessary parts, as indicated in prdm0/ropenblas#21. I also improved the benchmarks for building a presentable table. The benchmark plot now includes two algebraic routines for comparison. The new PDF of the paper can be found in the comment below.

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

pratikvn commented 3 years ago

@timtroendle, the paper looks good from my side.

timtroendle commented 3 years ago

Thank you @pratikvn.

prdm0 commented 3 years ago

@timtroendle , Do I have to do anything else for the paper to be finalized? Best regards.

timtroendle commented 3 years ago

Not at this point, thanks @prdm0 .

timtroendle commented 3 years ago

@myousefi2016, did you have a chance to look at the updates?

myousefi2016 commented 3 years ago

@timtroendle Yes, I checked and it looks good to me as well.

prdm0 commented 3 years ago

Dear @timtroendle, how long does it take for the paper to be listed on the JOSS page? Best regards.

timtroendle commented 3 years ago

@prdm0, as both reviewers are satisfied with the changes, we can start the acceptance process, which should not take long. At this point could you please do the following:

whedon commented 3 years ago

I'm sorry @prdm0, I'm afraid I can't do that. That's something only editors are allowed to do.

prdm0 commented 3 years ago

I'm sorry @prdm0, I'm afraid I can't do that. That's something only editors are allowed to do.

My language is not English. I must have run an unallowed command.

prdm0 commented 3 years ago

Dear @timtroendle ,

1 - https://github.com/prdm0/ropenblas/tree/v0.2.9, tag v0.2.9; 2 - https://zenodo.org/record/4618251#.YFOGiytv83w; 3 - Metadata is correct. I made the appropriate adjustments; 4 - https://doi.org/10.5281/zenodo.4618251, DOI: 10.5281/zenodo.4618251.

timtroendle commented 3 years ago

Dear @prdm0, thanks for this! Your submission is almost ready for publication. However, the paper needs more work. With more than 2000 words and 6 pages, it’s too long. Can you please shorten the paper to about 1000 words or less? Here are two possibilities:

First, the paper contains a lot of information taken from the documentation. This information is not required in the JOSS article. This concerns in particular the installation instructions and the function overview in the “Brief explanation” section. You could shorten this section drastically be limiting it to the most important function(s), or by discussing an example use only.

Second, the paper contains redundancy: the sections “Summary”, “Statement of Need”, and “Introduction” have quite some overlap. If you streamline these sections, you should be able to cut the number of words significantly. You could, for example, discuss the importance of computational efficiency and OpenBLAS in the Summary section; explain the benefits of ropenblas in the “Statement of Need” section; and reveal further details of ropenblas in a third section (likely not called “Introduction”). This is only a suggestion and I leave the details up to you.

prdm0 commented 3 years ago

Dear @timtroendle , I will proceed with the changes now.

prdm0 commented 3 years ago

Dear @timtroendle, I made the reductions so that there was a reduction of one page. I believe that it complies with the other papers in JOSS that have up to a maximum of 5 pages.

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

prdm0 commented 3 years ago

Dear @timtroendle, it decreases even more and the paper. The material now has less than 2000 words, based on the command I used, more precisely with 1998. But there are other ways of counting words that can provide different values. I compared it with the last published papers, using the same command and the number of words value is now below some.

[prdm0@samsung paper]$ pdftotext paper.pdf - | wc -w
1998
prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

prdm0 commented 3 years ago

@timtroendle, when possible, let me know according to the program you use to count words if the PDF above is suitable. I made a comparison with the latest publications, considering my way of telling, the paper has fewer words than those last publications.

I reduced it by a little more than one page. I believe that the paper is now suitable.

Best regards.

timtroendle commented 3 years ago

Thank you for the offer @prdm0, but this is not about the exact number of words. Rather, it's about having the right content in the article. Let me have a look at your most recent changes and come back to you soon.

timtroendle commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

timtroendle commented 3 years ago

@prdm0, I made some edits to your draft (attached). Can you please work these edits in? 10.21105.joss.02769-4-TT.pdf

I want to highlight the "Brief explanation" section here. As I mentioned above, large parts of this section are taken from the documentation and provide a level of detail that is not necessary for a JOSS article. Can you please remove or shorten this section? There is no need to discuss individual functions of ropenblas in this article.

prdm0 commented 3 years ago

@timtroendle I'll see how to do it. In the section Bifef explanation there is everything that was asked by the reviewers. But I can remove it.

prdm0 commented 3 years ago

I will remove all explanations of the functions and leave only the benchmark.

prdm0 commented 3 years ago

@timtroendle I made all the modifications. The PDF with the modified paper is below.

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

prdm0 commented 3 years ago

@whedon generate pdf

whedon commented 3 years ago

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

myousefi2016 commented 3 years ago

Hi @prdm0, just a quick comment here: please finalize all your changes to the manuscript before generating pdf. It's not necessary to do some incremental changes and then generate the pdf. I see there are lots of generate pdf commands here which makes this issue a bit lengthy and hard to follow. It is possible to avoid this by finalizing all your changes and just have one generate pdf command at the end.

prdm0 commented 3 years ago

Hi @prdm0, just a quick comment here: please finalize all your changes to the manuscript before generating pdf. It's not necessary to do some incremental changes and then generate the pdf. I see there are lots of generate pdf commands here which makes this issue a bit lengthy and hard to follow. It is possible to avoid this by finalizing all your changes and just have one generate pdf command at the end.

Dear @myousefi2016 I know that. However, the PDF was in a format here in RStudio and when the graphic was generated it was on another page. That's why it was generated. But don't worry, I know that and the last generation has been realized and is above your comment.