openjournals / joss-reviews

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

[REVIEW]: hawen: time-harmonic wave modeling and inversion using hybridizable discontinuous Galerkin discretization #2699

Closed whedon closed 3 years ago

whedon commented 3 years ago

Submitting author: @flofaucher (Florian Faucher) Repository: https://gitlab.com/ffaucher/hawen Version: 1.0.0 Editor: @meg-simula Reviewer: @chennachaos, @yangbai90 Archive: 11353/10.1144094

: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/aaac99edf0ce2d26a3e22b69558b0b55"><img src="https://joss.theoj.org/papers/aaac99edf0ce2d26a3e22b69558b0b55/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/aaac99edf0ce2d26a3e22b69558b0b55/status.svg)](https://joss.theoj.org/papers/aaac99edf0ce2d26a3e22b69558b0b55)

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

@chennachaos & @yangbai90, 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 @meg-simula 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 @chennachaos

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @yangbai90

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

whedon commented 3 years ago

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @chennachaos, @yangbai90 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/openjournals/joss-reviews) 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/openjournals/joss-reviews:

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 3 years ago

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

meg-simula commented 3 years ago

@yangbai90 @chennachaos Thanks again for agreeing to review. Checklists have been generated for each you above, ready for your review :-)

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

OK DOIs

- 10.1190/geo2019-0251.1 is OK
- 10.1093/gji/ggaa009 is OK
- 10.1051/m2an/2019009 is OK
- 10.1088/1361-6420/ab3507 is OK
- 10.1051/m2an/2019088 is OK
- 10.1016/j.jcp.2018.05.011 is OK
- 10.1137/15M1043856 is OK
- 10.1190/segam2016-13961828.1 is OK
- 10.1190/geo2019-0527.1 is OK
- 10.1137/S0036142901384162 is OK
- 10.1137/070706616 is OK
- 10.1007/s10915-011-9501-7 is OK
- 10.1137/S0895479899358194 is OK
- 10.1016/j.parco.2005.07.004 is OK
- 10.1007/b98874 is OK
- 10.1190/1.3238367 is OK
- 10.1046/j.1365-246X.1998.00498.x is OK
- 10.1046/j.1365-246x.1999.00967.x is OK

MISSING DOIs

- 10.1016/j.cma.2020.113406 may be a valid DOI for title: Adjoint-state method for Hybridizable Discontinuous Galerkin discretization:  application to the inverse acoustic wave problem
- 10.1190/geo2019-0251.1 may be a valid DOI for title: A priori estimates of attraction basins for velocity model reconstruction by time-harmonic Full Waveform Inversion and Data Space Reflectivity formulation
- 10.1142/7486 may be a valid DOI for title: Waves and rays in elastic continua

INVALID DOIs

- None
yangbai90 commented 3 years ago

@whedon commands

whedon commented 3 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
yangbai90 commented 3 years ago

@whedon check repository

whedon commented 3 years ago
Software report (experimental):

github.com/AlDanial/cloc v 1.84  T=1.05 s (274.4 files/s, 97134.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Fortran 90                     278          10052          29918          59981
C                                3            153            152            468
TeX                              1             52             16            360
Markdown                         4             62              0            352
make                             1              3             26             18
-------------------------------------------------------------------------------
SUM:                           287          10322          30112          61179
-------------------------------------------------------------------------------

Statistical information for the repository 'cba03b43192a8c3431ac8a62' was
gathered on 2020/10/06.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Florian Faucher                  2           773              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
Florian Faucher             773          100.0          0.0               19.79
meg-simula commented 3 years ago

@flofaucher @chennachaos @yangbai90 Just checking in to see how you are doing on this review? Do let me know if there is anything I can contribute with at this point.

flofaucher commented 3 years ago

@meg-simula Thanks, on my side, I guess I should wait for a formal reply before doing any modification is that correct? Or is there something I can/should start?

meg-simula commented 3 years ago

Correct, we are awaiting feedback from the reviewers at this stage.

chennachaos commented 3 years ago

Hi. I have been busy with some personal stuff. I hope to complete the review in 2 to 3 weeks from today.

yangbai90 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

- 10.1190/geo2019-0251.1 is OK
- 10.1093/gji/ggaa009 is OK
- 10.1051/m2an/2019009 is OK
- 10.1088/1361-6420/ab3507 is OK
- 10.1051/m2an/2019088 is OK
- 10.1016/j.jcp.2018.05.011 is OK
- 10.1137/15M1043856 is OK
- 10.1190/segam2016-13961828.1 is OK
- 10.1190/geo2019-0527.1 is OK
- 10.1137/S0036142901384162 is OK
- 10.1137/070706616 is OK
- 10.1007/s10915-011-9501-7 is OK
- 10.1137/S0895479899358194 is OK
- 10.1016/j.parco.2005.07.004 is OK
- 10.1007/b98874 is OK
- 10.1190/1.3238367 is OK
- 10.1046/j.1365-246X.1998.00498.x is OK
- 10.1046/j.1365-246x.1999.00967.x is OK

MISSING DOIs

- 10.1016/j.cma.2020.113406 may be a valid DOI for title: Adjoint-state method for Hybridizable Discontinuous Galerkin discretization:  application to the inverse acoustic wave problem
- 10.1190/geo2019-0251.1 may be a valid DOI for title: A priori estimates of attraction basins for velocity model reconstruction by time-harmonic Full Waveform Inversion and Data Space Reflectivity formulation
- 10.1142/7486 may be a valid DOI for title: Waves and rays in elastic continua

INVALID DOIs

- None
yangbai90 commented 3 years ago

@flofaucher : Hi Florian, it seems some DOIs are still missing, if these references are completed, on my side, it's ready for publication. BR Yang

flofaucher 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

- 10.1190/geo2019-0251.1 is OK
- 10.1093/gji/ggaa009 is OK
- 10.1051/m2an/2019009 is OK
- 10.1088/1361-6420/ab3507 is OK
- 10.1051/m2an/2019088 is OK
- 10.1016/j.jcp.2018.05.011 is OK
- 10.1137/15M1043856 is OK
- 10.1190/segam2016-13961828.1 is OK
- 10.1190/geo2019-0527.1 is OK
- 10.1016/j.cma.2020.113406 is OK
- 10.1137/S0036142901384162 is OK
- 10.1137/070706616 is OK
- 10.1007/s10915-011-9501-7 is OK
- 10.1137/S0895479899358194 is OK
- 10.1016/j.parco.2005.07.004 is OK
- 10.1007/b98874 is OK
- 10.1190/1.3238367 is OK
- 10.1142/7486 is OK
- 10.1046/j.1365-246X.1998.00498.x is OK
- 10.1046/j.1365-246x.1999.00967.x is OK

MISSING DOIs

- None

INVALID DOIs

- 10.1137/20M1336709 is INVALID
whedon commented 3 years ago
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1190/geo2019-0251.1 is OK
- 10.1093/gji/ggaa009 is OK
- 10.1051/m2an/2019009 is OK
- 10.1088/1361-6420/ab3507 is OK
- 10.1051/m2an/2019088 is OK
- 10.1016/j.jcp.2018.05.011 is OK
- 10.1137/15M1043856 is OK
- 10.1190/segam2016-13961828.1 is OK
- 10.1190/geo2019-0527.1 is OK
- 10.1016/j.cma.2020.113406 is OK
- 10.1137/S0036142901384162 is OK
- 10.1137/070706616 is OK
- 10.1007/s10915-011-9501-7 is OK
- 10.1137/S0895479899358194 is OK
- 10.1016/j.parco.2005.07.004 is OK
- 10.1007/b98874 is OK
- 10.1190/1.3238367 is OK
- 10.1142/7486 is OK
- 10.1046/j.1365-246X.1998.00498.x is OK
- 10.1046/j.1365-246x.1999.00967.x is OK

MISSING DOIs

- None

INVALID DOIs

- 10.1137/20M1336709 is INVALID
flofaucher commented 3 years ago

@whedon check references

flofaucher commented 3 years ago

@meg-simula @yangbai90 Thank you, I have just updated the bibliography. There are two preprints that have been accepted that I listed as "to appear", I can update with the appropriate doi as soon as I got them.

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

OK DOIs

- 10.1190/geo2019-0251.1 is OK
- 10.1093/gji/ggaa009 is OK
- 10.1051/m2an/2019009 is OK
- 10.1088/1361-6420/ab3507 is OK
- 10.1051/m2an/2019088 is OK
- 10.1016/j.jcp.2018.05.011 is OK
- 10.1137/15M1043856 is OK
- 10.1190/segam2016-13961828.1 is OK
- 10.1190/geo2019-0527.1 is OK
- 10.1016/j.cma.2020.113406 is OK
- 10.1137/S0036142901384162 is OK
- 10.1137/070706616 is OK
- 10.1007/s10915-011-9501-7 is OK
- 10.1137/S0895479899358194 is OK
- 10.1016/j.parco.2005.07.004 is OK
- 10.1007/b98874 is OK
- 10.1190/1.3238367 is OK
- 10.1142/7486 is OK
- 10.1046/j.1365-246X.1998.00498.x is OK
- 10.1046/j.1365-246x.1999.00967.x is OK

MISSING DOIs

- None

INVALID DOIs

- None
yangbai90 commented 3 years ago

@flofaucher : Great, now everything is ready. It can be published soon!

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

chennachaos commented 3 years ago

Hi @flofaucher. Please update the bibliography section of the paper. Update the paper in CMAME with the volume and page numbers, and remove the references that are not yet online in any form. If you have pre-prints for the ones that are "to appear", then add the link to the pre-print. Otherwise, remove them.

chennachaos commented 3 years ago

Hi @flofaucher. I have the following comments on the Documentation section.

1) Automated tests are missing. I suggest adding two automated tests. 2) Include information about contributing to the software, reporting bugs and seeking support. 3) Add equations for the visco-acoustic and visco-elastic problems. 4) Add a section on HDG, explaining briefly what HDG is, followed by a list of references on HDG. This will be useful for users who are not familiar with HDG. 5) Briefly discuss the current limitations of the methodology and the software so the users are aware of them before using the software. 6) Change mpi to MPI in every place it occurs. 7) Tutorial page: change "waves peed" to "wave speed". It is understandable that some typos always escape our attention. I suggest running the documentation through a spell and grammar checker once to identify and correct any potential typos.

I will check the installation and functionality parts once the above-listed issues are addressed.

flofaucher commented 3 years ago

Hello @chennachaos Thanks for your comments. Just to clarify, the modifications you suggest are for the Documentation section in the website right?  Or should I also update the JOOS preprint?  (for instance, the details on HDG and the equations were in the first version of the preprint that I had to shorten). 

Otherwise, I agree with all of your comments and I am going to update once I am sure it is the right place. Many thanks,

chennachaos commented 3 years ago

Hi @flofaucher. Yes. My first comment on references is for the JOSS paper only. Apart from the bibliography, the rest of the paper is fine. No need to add any equations to the paper.

All the modifications in my second comment are for the documentation section of the website. For the tests, you need to add the files to the repository, preferably as a separate folder. The steps to compile and run the tests should go into the installation part of the documentation (website).

flofaucher commented 3 years ago

Hi @chennachaos ,   I have updated the repository and website with the appropriate corrections, in particular,  

Please let me know if it clarifies your questions or if you suggest other modifications. 

Regarding the references in the Joss paper, I am waiting a bit as I expect to obtain the appropriate doi in the next few days. Nonetheless, I will modify it accordingly if I do not get them in time.

Many thanks,

meg-simula commented 3 years ago

@chennachaos @yangbai90 @flofaucher Just checking in - looks like there is good progress - thanks for all of your efforts!

chennachaos commented 3 years ago

Hi @flofaucher. Thank you for your updates. All the updates are to a satisfactory level.

I have checked the installation as well. It went smoothly. I also tested the code with the acoustic benchmark example in the tutorial section. The simulation ran successfully. The library is impressive. I would certainly recommend it to anybody working on wave propagation problems. I have the following minor issues that need to be addressed before I make my final recommendation. 1.) For the contour plots in the tutorial section, add the color bar legend so that it will be easy for the user to compare the results. 2.) Finalise the paper with the latest available details for the refernces.

flofaucher commented 3 years ago

Hi @chennachaos, Thank you for your comments and positive feedback. I have done the changes: 1.) color bars have been added in all of the pictures oft the Tutorial section 2.) I have updated the bibliography of the paper, in particular removing the ones that are not yet online.

I cc @meg-simula, please let me know if there is anything I can do now.

chennachaos commented 3 years ago

Hi @meg-simula. @flofaucher has responded promptly and addressed all the concerns. I am happy with the current state of the library. I recommend hawen for publication in the JOSS.

Thank you for opportunity!

meg-simula commented 3 years ago

Great, thanks @chennachaos and @yangbai90 for your prompt and constructive reviews and @flofaucher for addressing all the concerns. I will give it a final readthrough and follow-up from here.

meg-simula commented 3 years ago

@whedon generate pdf

meg-simula 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

- 10.1190/geo2019-0251.1 is OK
- 10.1093/gji/ggaa009 is OK
- 10.1051/m2an/2019009 is OK
- 10.1088/1361-6420/ab3507 is OK
- 10.1051/m2an/2019088 is OK
- 10.1016/j.jcp.2018.05.011 is OK
- 10.1137/15M1043856 is OK
- 10.1190/segam2016-13961828.1 is OK
- 10.1190/geo2019-0527.1 is OK
- 10.1016/j.cma.2020.113406 is OK
- 10.1190/geo2020-0305.1 is OK
- 10.1137/20M1336709 is OK
- 10.1137/S0036142901384162 is OK
- 10.1137/070706616 is OK
- 10.1007/s10915-011-9501-7 is OK
- 10.1137/S0895479899358194 is OK
- 10.1016/j.parco.2005.07.004 is OK
- 10.1007/b98874 is OK
- 10.1190/1.3238367 is OK
- 10.1142/7486 is OK
- 10.1046/j.1365-246X.1998.00498.x is OK
- 10.1046/j.1365-246x.1999.00967.x is OK

MISSING DOIs

- None

INVALID DOIs

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

meg-simula commented 3 years ago

@flofaucher Thanks for your patience here - this looks great!

At this point could you:

I can then move forward with accepting the submission.

flofaucher commented 3 years ago

@meg-simula Thank you, I am going to prepare the tagged version but I have a quick question before: Can it use the current version, or should it still be the one from last months (the current version does not have any added or removed functionality, only a few corrections to speedup operations).

meg-simula commented 3 years ago

Use the current version.

flofaucher commented 3 years ago

@meg-simula, I just created the tagged version and archived the version onto the repository phaidra (of Vienna University). It is available here: https://phaidra.univie.ac.at/o:1144094 and the handle doi is 11353/10.1144094 I am not sure if I have to indicate this information somewhere in an updated version of the paper now? Thanks!

meg-simula commented 3 years ago

@whedon set 11353/10.1144094 as archive

whedon commented 3 years ago

11353/10.1144094 doesn't look like an archive DOI.

meg-simula commented 3 years ago

@flofaucher Great, thanks! Could you confirm that the version is v1?

meg-simula commented 3 years ago

@openjournals/joss-eics This paper is ready for your attention.

Could you also advise on how to set the archive doi using whedon (cf. the above)? Aside from the archive and version setting, should be ready for whedon accept.

flofaucher commented 3 years ago

@meg-simula Yes the version is 1.0.0. I can ask phaidra for support regarding the doi too if you need to.

meg-simula commented 3 years ago

@whedon set 1.0.0 as version

whedon commented 3 years ago

OK. 1.0.0 is the version.

kthyng commented 3 years ago

@meg-simula @openjournals/dev I see that the DOI "11353/10.1144094" does link appropriately to what looks like a DOI with "doi.org/11353/10.1144094" so looks fine. How can we convince whedon of this, or skip over whedon to get it in the system appropriately?