openjournals / joss-reviews

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

[PRE REVIEW]: Unicycle: numerical simulations of seismic cycles in a viscoelastic half space with the integral method #5602

Closed editorialbot closed 2 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@sbarbot<!--end-author-handle-- (Sylvain Barbot) Repository: https://bitbucket.org/sbarbot/unicycle Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: !--editor-->@jedbrown<!--end-editor-- Reviewers: Pending Managing EiC: Kristen Thyng

Status

status

Status badge code:

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

Author instructions

Thanks for submitting your paper to JOSS @sbarbot. Currently, there isn't a JOSS editor assigned to your paper.

@sbarbot if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). You can search the list of people that have already agreed to review and may be suitable for this submission.

Editor instructions

The JOSS submission bot @editorialbot is here to help you find and assign reviewers and start the main review. To find out what @editorialbot can do for you type:

@editorialbot commands
editorialbot commented 1 year ago

Hello human, 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=1.55 s (455.6 files/s, 270611.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
XML                              4              0              0         175010
Fortran 90                     100          12075          16681          48025
MATLAB                         313           8504          17986          40270
TeX                              3           2703              1          25483
Fortran 77                      55           1965           9697          22501
Bourne Shell                   141           1855           2936          16517
C++                             24            831            760           5870
C/C++ Header                    39            631            924           3238
C                                7            336            626           1077
Markdown                         9            564              0            998
Perl                             1            108             44            499
make                             9            134             22            341
YAML                             1              3             22            108
-------------------------------------------------------------------------------
SUM:                           706          29709          49699         339937
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 511

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

OK DOIs

- 10.1016/j.tecto.2019.05.004 is OK
- 10.1029/2021JB023519 is OK
- 10.1038/nature19783 is OK
- 10.1186/s40623-021-01543-9 is OK
- 10.1186/s40623-020-01185-3 is OK
- 10.1186/s40623-020-1145-0 is OK
- 10.1002/2016GL070345 is OK
- 10.1029/2019GC008515 is OK
- 10.1785/0120160237 is OK
- 10.1785/0120180058 is OK

MISSING DOIs

- 10.1029/2022jb024069 may be a valid DOI for title: Contribution of viscoelastic stress to the synchronization of earthquake cycles on oceanic transform faults
- 10.1016/b978-012387582-2/50038-1 may be a valid DOI for title: Paraview: An end-user tool for large data visualization

INVALID DOIs

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

kthyng commented 1 year ago

Hi @sbarbot and thanks for your submission. I am going to add it to our waitlist as we need a relevant editor with some availability. In the meantime, please take a look at the suggested missing DOIs above, in case they are relevant. Thanks for your patience.

sbarbot commented 1 year ago

@kthyng I have updated the DOI number for Shi et al. (2022). However, there is no DOI available for the Paraview handbook, which is a collection of papers.

sbarbot commented 1 year ago

@kthyng The pre-print of the companion paper "Numerical simulations of seismic cycles in a viscoelastic half-space with the integral method" submitted at J. Geophys. Res. is available at the Earth and Space Science Open Archive.

kthyng commented 1 year ago

@jedbrown I want to ping you here and get an invite out to you while I see you have a slot in your editor allocation! Also I'm going out of town later today. Might you be able to take this submission on?

kthyng commented 1 year ago

@editorialbot invite @jedbrown as editor

editorialbot commented 1 year ago

Invitation to edit this submission sent!

sbarbot commented 1 year ago

Hi All, any update with this submission?

jedbrown commented 1 year ago

Sorry about the delay; I've been buried and need to decline more service requests. I'm willing to take this, but I'm going to ask for some build and design improvements to support "maintainable extension". This may not be universal, but I feel a clear pattern that JOSS submissions developed in languages other than Fortran have made leaps of progress on composition/usability/packaging over the past few years, and yet here we have lots of near-duplicate code in different directories with copy/modify-the-makefile and limited portability testing. You would not be wrong to say that these practices are accepted by many Fortran programmers, but as someone who wrote a paper on extensibility and "librarization" of simulation software and spends a lot of time dealing with externalities of such practices, it is a bit of a pet issue for me. I understand you may not want to make such changes (and I would understand that choice), but I'd ask that we find a different editor if that is the case.


BTW, I get loads of errors building with gfortran-13 (type and rank mismatches).

I think it's great that you wrote man pages, though "when unicycle/man is in the MANPATH" must be "when PATH_TO_UNICYCLE/fortran/man is in the MANPATH" (adopting a convention I see in hmmvp/readme.txt, though I don't know if you want to keep that).

sbarbot commented 1 year ago

@jedbrown, thank you for your thoughtful comments.

I fixed the documentation accordingly. I can work on a CMake configure/make framework to make the installation more portable. I see the merit in "Librarizing" these codes, but it would have to be for the next version, as it requires substantial efforts that are beyond the scope of this initial submission.

Considering where my community is at this point, the merit of new codes is first and foremost to provide functionality. For example, the mathematics underlying the numerical method took several years to develop. See this example, that example, and the companion paper. I see that other codes, such as pylith, introduced "librarization" in subsequent versions.

I appreciate your comments as an editor. I've learned a lot from you.

sbarbot commented 1 year ago

BTW, I get loads of errors building with gfortran-13 (type and rank mismatches).

Are these errors or warnings? I also get a lot of warnings for type mismatches for the MPI library for functions like MPI_BCAST. I'm not sure I can fix that.

jedbrown commented 1 year ago

They are errors. I think you can suppress the error with compiler flags and there are local changes you can make in places (e.g., to handle the rank mismatch errors), but the MPI Forum would say you should use mpi_f08:

image

sbarbot commented 1 year ago

Thank you. I updated the code with USE mpi_f08. I removed the -fallow-argument-mismatch compiler option. Using mpi_f08 did not create any issue with this code. However, I tried on other codes and it created problems. For example, the following program does not compile.

 PROGRAM test

  USE mpi_f08

  INTEGER, DIMENSION(2) :: vector

  INTEGER :: num_proc,ierr

  CALL MPI_INIT(ierr)
  CALL MPI_COMM_SIZE(MPI_COMM_WORLD, num_proc, ierr)
  CALL MPI_TYPE_COMMIT(vector,ierr)
  CALL MPI_FINALIZE(ierr)

END PROGRAM test
jedbrown commented 1 year ago

That's because vector has the wrong type. Change its declaration as below and the compiler will accept it (the code is wrong because no type is created here).

  TYPE(MPI_Datatype) :: vector
sbarbot commented 1 year ago

Thank you. Problem solved. I updated the software Motorcycle, which you also oversee as an editor, accordingly.

I will create the CMake installation procedure on September 9, during the CIG-led workshop for community software.

kthyng commented 1 year ago

My understanding is that @jedbrown will be the editor for this submission, but it is paused until another submission is finalized. I will add these two changes but let me know if I'm wrong.

kthyng commented 1 year ago

@editorialbot add @jedbrown as editor

editorialbot commented 1 year ago

Assigned! @jedbrown is now the editor

crvernon commented 5 months ago

:wave: @jedbrown and @sbarbot - are we ready to start the review on this one? It has been paused since September last year. Please let me know how you would like to continue. Thanks!

arfon commented 2 months ago

@editorialbot reject

editorialbot commented 2 months ago

Paper rejected.