search

JuliaCon / proceedings-review

6 stars 1 forks source link

[REVIEW]: Adaptive numerical simulations with Trixi.jl: A case study of Julia for scientific computing #77

Closed whedon closed 2 years ago

whedon commented 2 years ago

Submitting author: !--author-handle-->@ranocha<!--end-author-handle-- (Hendrik Ranocha) Repository: https://github.com/trixi-framework/paper-2021-juliacon Branch with paper.md (empty if default branch): Version: Editor: !--editor-->@carstenbauer<!--end-editor-- Reviewers: @johnfgibson, @simonbyrne Archive:

Status

status

Status badge code:

HTML: <a href="https://proceedings.juliacon.org/papers/e0a710cb74904fbd77e6528d4b55c7ce"><img src="https://proceedings.juliacon.org/papers/e0a710cb74904fbd77e6528d4b55c7ce/status.svg"></a>
Markdown: [![status](https://proceedings.juliacon.org/papers/e0a710cb74904fbd77e6528d4b55c7ce/status.svg)](https://proceedings.juliacon.org/papers/e0a710cb74904fbd77e6528d4b55c7ce)

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

@johnfgibson & @simonbyrne, 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 @carstenbauer 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 @johnfgibson

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Paper format

Content

Code of Conduct

General checks

Functionality

Documentation

Paper format

Content

whedon commented 2 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @johnfgibson, @simonbyrne it looks like you're currently assigned to review this paper :tada:.

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

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/JuliaCon/proceedings-review) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

  1. Set yourself as 'Not watching' https://github.com/JuliaCon/proceedings-review:

watching

  1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

notifications

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

@whedon commands

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

@whedon generate pdf
whedon commented 2 years ago

Wordcount for paper.tex is 6335

whedon commented 2 years ago 
Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.06 s (574.7 files/s, 297581.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TeX                              8            390            233          11260
TOML                             8            919              4           4118
Markdown                         7             55              0            355
Julia                            5            120             91            261
INI                              3             18              0            136
Ruby                             1              8              4             45
YAML                             1              0              0             44
JSON                             1              1              0             38
make                             1              3              0             20
-------------------------------------------------------------------------------
SUM:                            35           1514            332          16277
-------------------------------------------------------------------------------

Statistical information for the repository '32f94ca8555bbe2f47636557' was
gathered on 2021/09/13.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Hendrik Ranocha                  1            57              0          100.00

Below are the number of rows from each author that have survived and are still
intact in the current revision:

Author                     Rows      Stability          Age       % in comments
Hendrik Ranocha              57          100.0          0.0                7.02
whedon commented 2 years ago

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

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

OK DOIs

- 10.5281/zenodo.5201484 is OK
- 10.1016/j.jcp.2021.110467 is OK
- 10.1007/s42967-021-00148-z is OK
- 10.1137/141000671 is OK
- 10.5334/jors.151 is OK
- 10.21105/jcon.00068 is OK
- 10.21105/joss.03454 is OK
- 10.1137/15M1020575 is OK
- 10.1109/HPTCDL.2014.10 is OK
- 10.21105/joss.03053 is OK
- 10.21105/joss.02520 is OK
- 10.21105/joss.02018 is OK
- 10.1016/j.camwa.2020.05.004 is OK
- 10.1016/j.jcp.2020.109844 is OK
- 10.1137/100791634 is OK
- 10.1038/467753a is OK
- 10.1093/biostatistics/kxq028 is OK
- 10.1109/TPDS.2018.2872064 is OK
- 10.1093/mnras/stu114 is OK
- 10.1007/978-0-387-72067-8 is OK
- 10.1007/978-90-481-2261-5 is OK
- 10.1090/S0025-5718-1987-0890255-3 is OK
- 10.1137/S003614290240069X is OK
- 10.1016/j.jcp.2013.06.014 is OK
- 10.1007/s10915-017-0618-1 is OK
- 10.1016/j.jcp.2017.05.025 is OK
- 10.1016/S0021-9991(03)00168-2 is OK
- 10.1137/18M1209234 is OK
- 10.1142/9789814313193_0004 is OK
- 10.18419/opus-3895 is OK
- 10.1016/j.jcp.2018.06.027 is OK
- 10.1007/b79761 is OK
- 10.1016/j.jcp.2004.08.020 is OK
- 10.1007/978-3-030-39647-3_42 is OK
- 10.1016/j.jcp.2016.09.013 is OK
- 10.18419/opus-3788 is OK
- 10.1109/PMBS51919.2020.00008 is OK
- 10.1145/2464996.2465020 is OK
- 10.1145/2807591.2807623 is OK

MISSING DOIs

- None

INVALID DOIs

- None
ranocha commented 2 years ago

As mentioned in the pre-review issue #76, the software report is not really helpful since we set up a dedicated repository with reproducibility material for this paper. Our Julia package Trixi.jl is available in another repository.

whedon commented 2 years ago

:wave: @simonbyrne, please update us on how your review is going (this is an automated reminder).

whedon commented 2 years ago

:wave: @johnfgibson, please update us on how your review is going (this is an automated reminder).

carstenbauer commented 2 years ago

@simonbyrne @johnfgibson: kind reminder here.

simonbyrne commented 2 years ago

I've started on the review, however it might be better to move the paper to a branch of the main Trixi.jl repository (which I think is what the guidelines suggest to do) so that the links all work correctly when published, e.g. the "Software repository" link on the published version: https://proceedings.juliacon.org/papers/10.21105/jcon.00068

simonbyrne commented 2 years ago

From the guide for authors:

On the technical side, the submission (to be submitted through this form) must be based on a git repository on GitHub. Typically, this would be the repository of your julia package or code. The paper itself should be written in LaTeX (not Markdown) and should reside in a paper/ subfolder (potentially in a separate paper branch) of this repository.

I think this is also required for the Zenodo link to work correctly.

ranocha commented 2 years ago

Thank you very much for investing your time to help us improving our paper, @simonbyrne. We thought about the location of our paper draft based on the guide for authors reproduced below.

image

Since the highlighted note explicitly states

It is possible to have the paper in a separate repository

we decided to use this option. Our main reason for not putting the paper draft in the Trixi.jl repository is that it would considerably increases the size of our main repo. The paper repo is already ca. 5 MB large and that's without possible iterations during the review. Deleting the branch after publication is also not an option for us, since we specifically want to have a reproducibility repository that is permanent. Also, this does not scale if we want to submit another paper using a similar submission process based on Trixi.jl.

Thus, we would like to keep the paper draft as a separate repo to not clutter Trixi.jl. At the same time, this paper repo is designed as a permanent reproducibility repository like we have for all our papers. We hope that the link to the software repository can be changed by hand upon publication.

carstenbauer commented 2 years ago

@simonbyrne @johnfgibson: another kind reminder here. Would be great to get some (initial) feedback soon. Thanks in advance!

simonbyrne commented 2 years ago

Apologies, I will get this done ASAP.

carstenbauer commented 2 years ago

@simonbyrne @johnfgibson: Happy New Year! How is your review going? Given that the review process has been started quite some time ago it would be great if you could finish your review by the end of next week.

(If, due to unforeseen reasons, you can't finish your review within this time frame, please let me know ASAP!)

johnfgibson commented 2 years ago

@carstenbauer: Thanks for the reminder. I'm on it and will get it done by end of next week if not earlier.

johnfgibson commented 2 years ago

@whedon commands

whedon commented 2 years ago

Here are some things you can ask me to do:

# List Whedon's capabilities
@whedon commands

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

EDITORIAL TASKS

# Compile the paper
@whedon generate pdf

# Compile the paper from alternative branch
@whedon generate pdf from branch custom-branch-name

# Ask Whedon to check the references for missing DOIs
@whedon check references

# Ask Whedon to check repository statistics for the submitted software
@whedon check repository
whedon commented 2 years ago

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

johnfgibson commented 2 years ago

@carstenbauer : Has my invitation to review been revoked? When click on https://github.com/openjournals/joss-reviews/invitations , I get the error message "Sorry, we couldn't find that repository invitation. It is possible that the invitation was revoked or that you are not logged into the invited account." I am logged on to my github account.

ranocha commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

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

ranocha commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

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

whedon commented 2 years ago

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

carstenbauer commented 2 years ago

@johnfgibson Thanks for your review! Regarding your invitation, can you please check https://github.com/JuliaCon/proceedings-review/invitations? The system says that there is a pending invitation for you.

johnfgibson commented 2 years ago

You're welcome! I'm still getting the "Sorry, we couldn't find that repository invitation...." error when I access that link. I am logged in as @johnfgibson.

github-invitation

carstenbauer commented 2 years ago

I tried to sent out a new invitation. Please try again.

johnfgibson commented 2 years ago

Got it! It works. Thanks. I am done with the review except for a verifying the performance claims in the paper, required by the checklist.

simonbyrne commented 2 years ago

@carstenbauer I also need an updated invite.

simonbyrne commented 2 years ago

I have submitted my review here: https://github.com/trixi-framework/paper-2021-juliacon/issues/22

carstenbauer commented 2 years ago

@carstenbauer I also need an updated invite.

Hm, are you sure? You have the "Member" label attached to your posts here and internally it also says:

Screenshot 2022-01-07 at 11 35 43

(If things are really not working for you I'll re-add you as a Member.)

whedon commented 2 years ago

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

whedon commented 2 years ago

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

whedon commented 2 years ago

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

ranocha commented 2 years ago

Sorry for the noise. Something appears to be strange with the old LaTeX installation of whedon. Our revisions based on the reviewers' comments increased the length of the manuscript slightly. The old LaTeX installation of whedon uses 11 pages with only a single reference on the last page. Locally on my computer, the layout is better and we get only 10 pages, see paper.pdf. I tried to get the same output from whedon but was not successful.

whedon commented 2 years ago

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

whedon commented 2 years ago

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

simonbyrne commented 2 years ago

@carstenbauer ah, i misunderstood what was involved. never mind, thanks!

ranocha commented 2 years ago

@whedon generate pdf

whedon commented 2 years ago

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

ranocha commented 2 years ago

Thanks again to the reviewers. We improved the manuscript based on the suggestions of @johnfgibson and @simonbyrne. The new manuscript built by whedon hits 11 pages. As described above, I get only 10 pages locally, see paper.pdf. It looks like some float placement logic changed from the (rather old) LaTeX installation of whedon to the (newer) version I use locally:

$ pdflatex --version
pdfTeX 3.14159265-2.6-1.40.20 (TeX Live 2019/Debian)
kpathsea version 6.3.1
Copyright 2019 Han The Thanh (pdfTeX) et al.
There is NO warranty.  Redistribution of this software is
covered by the terms of both the pdfTeX copyright and
the Lesser GNU General Public License.
For more information about these matters, see the file
named COPYING and the pdfTeX source.
Primary author of pdfTeX: Han The Thanh (pdfTeX) et al.
Compiled with libpng 1.6.37; using libpng 1.6.37
Compiled with zlib 1.2.11; using zlib 1.2.11
Compiled with xpdf version 4.01
ranocha commented 2 years ago

@carstenbauer What's your opinion on the page limit issue?

simonbyrne commented 2 years ago

I have completed my review as well.

I haven't personally run the performance benchmarks either, but I did verify the code to reproduce the results is in the paper repository, and that their methodology seems appropriate.

carstenbauer commented 2 years ago

Is it okay to have a few lines on page 11 (although that looks bad)?

If we can't fix it, it's fine IMO. But I'd like to fix it.

Is there any chance to update the LaTeX installation you use for this journal?

Not sure. We use the same (in the sense of identical) system as JOSS AFAIK. So updating the installation, if possible would probably need to go through @arfon.

Or do we need to find a way to convince whedon to generate the same layout I get locally? If so, is there any better way to do this than pushing to our repo and asking whedon to generate a new PDF here?

I don't see a better way, unfortunately.

ranocha commented 2 years ago

Okay, I will try again. Sorry for all the noise but that's what @carstenbauer said...

ranocha commented 2 years ago

@whedon generate pdf from branch hr/convince_whedon

whedon commented 2 years ago 
Attempting PDF compilation from custom branch hr/convince_whedon. Reticulating splines etc...
whedon commented 2 years ago

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

ranocha commented 2 years ago

@whedon generate pdf from branch hr/convince_whedon