[PRE REVIEW]: Motorcycle: A spectral boundary-integral method for seismic cycles on multiple faults

[editorialbot]

Submitting author: !--author-handle-->@sbarbot<!--end-author-handle-- (Sylvain Barbot) Repository: https://bitbucket.org/sbarbot/motorcycle/src/master/ Branch with paper.md (empty if default branch): Version: v1.0.0 Editor: !--editor-->@jedbrown<!--end-editor-- Reviewers: @mherman09, @willic3 Managing EiC: Kyle Niemeyer

Status

Status badge code:

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

Author instructions

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

The author's suggestion for the handling editor is @jedbrown.

@sbarbot if you have any suggestions for potential reviewers then please mention them here in this thread (without tagging them with an @). In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission (please start at the bottom of the list).

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]

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]

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1785/0120210004 is OK
- 10.1016/j.tecto.2019.05.004 is OK
- 10.1029/2021JB023519 is OK
- 10.1016/j.epsl.2021.117037 is OK
- 10.1016/j.epsl.2022.117593 is OK

MISSING DOIs

- None

INVALID DOIs

- None

[editorialbot]

Software report:

github.com/AlDanial/cloc v 1.88  T=0.08 s (782.7 files/s, 273582.1 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
Fortran 90                      29           2956           3577           9983
Bourne Shell                    22            223            404           2549
Perl                             1            108             44            499
Markdown                         4            178              0            255
YAML                             1              2             13             53
TeX                              1              5              0             50
MATLAB                           1              8              2             33
Fortran 77                       1              0              9             21
-------------------------------------------------------------------------------
SUM:                            60           3480           4049          13443
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

[editorialbot]

Wordcount for paper.md is 427

[editorialbot]

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

[kyleniemeyer]

Hi @jedbrown, can you edit this submission?

[kyleniemeyer]

@editorialbot invite @jedbrown as editor

[editorialbot]

Invitation to edit this submission sent!

[sbarbot]

Hi All,

Here are suggestions for reviewers:

• Brittany Erickson (University of Oregon)
• Junle Jiang (University of Oklahoma)
• Luca Dal Zilio (ETH Zurich)
• Pierre Romanet (NEID, Japan)
• Elias Heimisson (ETH Zurich)
• Brad Aagaard (USGS)
• Charles Williams (GNS Science)
• Matthew Knepley (University of Buffalo)
• Pierre Dublanchet (MINES ParisTech)

Best Regards, Sylvain Barbot

---

From: Kyle Niemeyer @.> Sent: Tuesday, July 26, 2022 1:01:06 AM To: openjournals/joss-reviews @.> Cc: Sylvain Barbot @.>; Mention @.> Subject: Re: [openjournals/joss-reviews] [PRE REVIEW]: Motorcycle: A spectral boundary-integral method for seismic cycles on multiple faults (Issue #4603)

@editorialbothttps://urldefense.com/v3/__https://github.com/editorialbot__;!!LIr3w8kk_Xxm!ujScZRtwNfwkJkTCl0HyTCIwPq4_cTKJjfS3TDBF3VTI902up2iblE6OX7egoqPxGxqr4yaY6JZmOjvU351WyRI$ invite @jedbrownhttps://urldefense.com/v3/__https://github.com/jedbrown__;!!LIr3w8kk_Xxm!ujScZRtwNfwkJkTCl0HyTCIwPq4_cTKJjfS3TDBF3VTI902up2iblE6OX7egoqPxGxqr4yaY6JZmOjvU-AAstZ8$ as editor

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/openjournals/joss-reviews/issues/4603*issuecomment-1194283399__;Iw!!LIr3w8kk_Xxm!ujScZRtwNfwkJkTCl0HyTCIwPq4_cTKJjfS3TDBF3VTI902up2iblE6OX7egoqPxGxqr4yaY6JZmOjvUvyDX1uw$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ABJHTWGYIY7DEUNLTRC6HSDVV224FANCNFSM54SZPJPA__;!!LIr3w8kk_Xxm!ujScZRtwNfwkJkTCl0HyTCIwPq4_cTKJjfS3TDBF3VTI902up2iblE6OX7egoqPxGxqr4yaY6JZmOjvUWjHWWBM$. You are receiving this because you were mentioned.Message ID: @.***>

[Kevin-Mattheus-Moerman]

@jedbrown :wave:

[jedbrown]

I can edit this, but I may not be able to work on it until Monday due to family commitments this week.

[Kevin-Mattheus-Moerman]

@jedbrown that is grand, thanks for helping!

[Kevin-Mattheus-Moerman]

@editorialbot assign @jedbrown as editor

[editorialbot]

Assigned! @jedbrown is now the editor

[jedbrown]

@sbarbot Sorry about my longer-than-expected delay in returning to this. I notice one missing reference, and would ask that you please include comparison to related software (along whatever dimensions you consider to be salient) and a figure demonstrating an example of the capability.

[jedbrown]

@sbarbot Also, am I missing some sort of quick-start on how to install and run in the documentation? I think this project needs at least a high-quality README (could be a wiki). Your pipeline yaml kind of has how to install prereqs, build, and run, but it should be somewhere meant to be read by humans. It should be enough for a scientist to follow the steps and interpret results as being physically meaningful in some sense (by visualization or diagnostic output).

[sbarbot]

@jedbrown I have updated the draft with a reference to other work and a new figure illustrating a typical model output. I have updated the tutorial examples so they automatically produce meaningful figures if GMT5 and above and/or GNUPLOT are installed. A script is available to reproduce Figure 1 in the paper.

I have updated the README file and added INSTALL.md to describe how to compile, run, and visualize the model.

Please see the updated repository.

[jedbrown]

@editorialbot generate pdf

[editorialbot]

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

[jedbrown]

Thank you for the updates and sorry about my slow handling. The paper looks much better. I have two remaining concerns:

1. The build is still quite manual and hard to maintain. For example, it seems likely that the various makefile_* files will get out of sync. A different approach would be to have one makefile that includes a machine-specific file (perhaps determined using $(shell uname) or $(MAKE_HOST)) or make machine-specific files (for make -f makefile_linux) that include the machine-independent stuff. The point is that machine-specific configuration should just be a few essential variables, and the rules should only ever be in one place.
2. The second concern is with respect to the criteria

   designed for maintainable extension

As far as I can tell from the examples in the repository, the extension mechanism is to copy the entire source tree and edit to make a new case. This all but prevents growing an ecosystem in which people can actively work on numerics (which would impact all cases) while others develop new cases. I'd encourage you to think (and we can discuss here) how you could refactor for "maintainable extension", so that these research activities could proceed concurrently. This would enable the software to have much more lasting impact.

[sbarbot]

@jedbrown I have updated the makefiles to have machine-specific files for make -f makefile_ubuntu that include a common.mk file that contains the machine-independent instructions. You are right that it will make the maintenance of makefiles for different machines more straightforward. Some examples were in fact already out of date. That has been fixed.

For maintainable extension that allows the separation of the common numerics and more specific applications, we already have a system in place based on a combination of pre-compiler directives that allow adding more global and local arrays and command-line options that affect the entire calculation.

For new specific applications, I envision a type of extension involving the integration of new analytic solutions. There is an example of this for the simulation of seismic cycles on faults embedded in a compliant zone instead of a homogeneous half-space. This simply requires calling a different function for the stress calculation and providing more input parameters. This extension has been implemented for the work of Nie & Barbot (Rupture styles linked to recurrence patterns in seismic cycles with a compliant fault zone, 2022). More similar cases can be added in the future, but not before some tedious work to derive these mathematical expressions in the first place.

Another expected extension is to implement different friction laws. While they may have a huge impact on the simulations, they represent very modest changes to the code that can be dealt with using command-line arguments and different calculations decided at run time. This is already implemented for the Ruina (1983), Rice & BenZion (1996), and Barbot (2019) friction law (and evolution laws). This type of modification is likely the most frequent extension of the code because there are already many different constitutive laws that researchers use, so implementing a new one is simply a matter of opportunity (no new research is needed).

Given the type of applications that this code enables, I believe there is already a system that provides sufficient flexibility for extensions. That said, I'd be happy to hear your suggestions.

[kthyng]

@jedbrown Would you be able to return to this submission?

[jedbrown]

Whoops, I'm very sorry about this falling off my plate. Thanks for your updates and explanation above.

As an example of maintainability, we have all these files that have lots of similarities.

$ wc **/ratestate*.f90
  1603   4431  51280 2d/antiplane/src-serial/ratestate_bath.f90
  1444   4020  45240 2d/antiplane/src-serial/ratestate_cz.f90
  1530   4183  48330 2d/antiplane/src-serial/ratestate.f90
  1736   4636  56326 2d/planestrain/src-serial/ratestate.f90
  1707   4645  55927 3d-serial/src/ratestate.f90
  2571   7047  95976 3d/src/ratestate_bath.f90
  2298   6357  86097 3d/src/ratestate.f90
 12889  35319 439176 total

To pick ratestate_cz.f90 and ratestate.f90, they are nearly copies, but have diverged (one has --export-state and --import-state, the other has --source-export-rate, they have different years for the citation to Barbot, 2018 versus Barbot, 2019. Every one of those six files has multiple case statements for every friction law (of which there are currently three). What happens if various members of the community start contributing friction laws while others set up different models (_cz, _bath, etc.)? How would you keep those models consistent? Or do you allow them to diverge and have separate documentation, and then using feature X in model Y becomes a development effort? What about MPI support? There's a lot of MPI code in 3d/src/ratestate.f90 and 3d/src/ratestate_bath.f90, but it seems MPI can't be used for 2D models at all.

I know this is common enough for internal research group projects, where each student forks the project and adds some features for their publications. But this mode disincentivizes contributing back to the community, and thus the project tends to never grow a developer community. A different approach is to use types and functions so that any friction law can be used with any fault configuration without combinatorial amounts of code. This is the kind of thing that moves a package from an individual research tool to community software.

I'm not going to insist on this change (reviewers might), but I think you would make the review more streamlined and significantly increase likelihood of community contributions to modularize in this way. Please let me know your preference.

At a more pedestrian level, usually makefiles use variables for all the flags, so that one can do make F90=gfortran instead of needing to edit several places where gfortran-9 is hard-coded alongside various flags.

[sbarbot]

The files with _cz and _bath are not part of this submission. In the future, we can implement these functionalities following your suggestions, allowing different friction laws and stress interactions within the same overall code and file.

I believe that the variables F77 and F90 in the make files are not hardwired but simply provided a default value. These variables can still be updated at runtime, i.e., at the command line, using, for example, make F77=gfortran-9 F90=gfortran-9. These values will bypass the ones in the make file. I think the submission is ready for peer review.

[jedbrown]

I hadn't seen noticed any previous statements about what was meant for review and what should be excluded. Are the _cz and _bath files the only ones that should be excluded?

Regarding the makefiles, you frequently use patterns like this

F77=gfortran-9 -cpp
F90=gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp

which means that one needs to consult the makefile and then run

$ make F77='gfortran -cpp' F90='gfortran -cpp -Wall -DNETCDF -DODE45 -fopenmp'

which is a lot for a user to remember (and get wrong and either complain to you or just move on and not recommend your software), and it's different in different subdirectories of your code. If you check make --print-data-base, you'll see recommended practice

COMPILE.F = $(FC) $(FFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c

.F.o:
#  Builtin rule
#  Implicit rule search has not been done.
#  Modification time never checked.
#  File has not been updated.
#  recipe to execute (built-in):
        $(COMPILE.F) $(OUTPUT_OPTION) $<

With these points addressed, I'll recruit reviewers. Thanks for your patience.

[sbarbot]

That is very useful, thank you. I have updated all the make files to include the COMPILE.F, LINK.F, RM, FFLAGS, and CFLAGS variables. This simplifies the building process greatly. The individual make files provide examples of how to fill these variables for various systems.

Formally, only the codes that are mentioned in the README (motorcycle-ap-ratestate motorcycle-ps-ratestate, motorcycle-3d-ratestate) and all the other forms of documentation are part of the submission.

Let me know if I have addressed your remaining concerns.

[jedbrown]

Thanks and sorry once again for my slow response. I see parallel make does not work (should either fix dependencies or use .NOTPARALLEL). I have to manually mkdir build otherwise I get

$ make -f makefile_ubuntu motorcycle-ps-ratestate-serial -j1
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/getopt_m.f90 -o build/getopt_m.o -J build
src-serial/getopt_m.f90:233:19:

  233 | end module getopt_m
      |                   1
Fatal Error: Cannot open module file ‘build/getopt_m.mod0’ for writing at (1): No such file or directory
compilation terminated.
make: *** [common.mk:14: build/getopt_m.o] Error 1

If you'd like to make the directory automatically, you can use this technique. After mkdir build, I get this warning, which is probably harmless but perhaps not necessary.

$ make -f makefile_ubuntu motorcycle-ps-ratestate-serial -j1
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/getopt_m.f90 -o build/getopt_m.o -J build
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/types.f90 -o build/types.o -J build
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch -c src-serial/getdata.f -o build/getdata.o
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/greens.f90 -o build/greens.o -J build
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/exportnetcdf.f90 -o build/exportnetcdf.o -J build
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/fft.f90 -o build/fft.o -J build
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/ode45.f90 -o build/ode45.o -J build
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch   -c  -c src-serial/ratestate.f90 -o build/ratestate.o -J build
gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp -I/usr/local/include -I/usr/include -finit-local-zero -ffree-line-length-none -O3 -ffast-math -w -Wargument-mismatch    -o build/motorcycle-ps-ratestate-serial build/getopt_m.o build/types.o build/getdata.o build/greens.o build/exportnetcdf.o build/fft.o build/ode45.o build/ratestate.o -L/usr/local/lib -lnetcdff -lfftw3_omp -lfftw3 -lm
/usr/bin/ld: warning: build/ratestate.o: requires executable stack (because the .note.GNU-stack section is executable)

I can run with --help, but --dry-run or otherwise hangs indefinitely, with the error after I press enter.

$ build/motorcycle-ps-ratestate-serial --help
usage:

OMP_NUM_THREADS=2 motorcycle-ps-ratestate-serial [-h] [--dry-run] [--help] [--epsilon 1e-6] [filename]
                                         [--export-netcdf] [--export-stress] [--source-export-rate 50]
[...]
$ build/motorcycle-ps-ratestate-serial --dry-run
# -----------------------------------------------------------------------------------
# MOTORCYCLE
# quasi-dynamic earthquake simulation in three-dimensional media
# with the spectral boundary integral method.
# friction law: multiplicative form of rate-state friction (Barbot, 2019)
# numerical accuracy:      1.0000E-06
# maximum iterations:         1000000
# maximum time step:       1.7977+308
# export velocity to netcdf:       no
# number of threads:          128/128
# -----------------------------------------------------------------------------------
# output directory

ERROR 102 at line 01223
error: unable to access /time.dat
STOP 1

When I try examples/tutorials/run1.sh, there is an error because Debian/Ubuntu systems don't ship with time as a command (but it is a shell keyword). Swapping OMP_NUM_THREADS=2 time ... for time OMP_NUM_THREADS=2 ... works.

I notice that the makefiles still have

FC=gfortran-9 -cpp -Wall -DNETCDF -DODE45 -fopenmp

so make FC=gfortran-11 does not work (in confusing ways). I think you should use something like:

FC = gfortran-9
FFLAGS = -cpp -Wall -fopenmp  # plus others as needed
CPPFLAGS = -DNETCDF -DODE45

So (a) the installation issues I've encountered are solved, but (b) nobody else should need to deal with this list of undocumented issues. I think you should fix these, but I'm ready to move forward.

---

Regarding reviewers, I see that you are a coauthor within the last year with the first four suggestions. (JOSS and other journals consider this a CoI.) Can you confirm that the other suggestions are not conflicted to your knowledge and let me know if any of the reviewers on this list would be a good fit?

[sbarbot]

Because the compilation is very short, I simply use .NOTPARALLEL. I now automatically create the build directory using $(shell mkdir -p $(DST)) in the common.mk file. The warning /usr/bin/ld may be addressed with -z noexecstack but this seems to depend on the version of ld. I left the warning. I now clearly separate FC and FFLAGS. I removed the time commands in the examples.

Running the code with --dry-run requires an input file, which is provided by the standard input if a file is not provided in the command line. This is why the code is hanging: it simply waits for the user to provide the output directory. If you simply press enter, the directory is empty, leading to a system error. Best to run a provided input file and use --dry-run as an additional option. This will read the input file and create some diagnostic output files, but there will not be any complex calculations. This option is to check the distribution of physical parameters before running lengthy simulations. This is a feature, not a bug.

The suggested reviewers are co-authors in a paper showing simulation benchmarks, part of the Southern California Earthquake Center initiative called Sequence of Earthquakes and Aseismic Slip. A large part of the international seismic cycle modeling community is present as co-authors on these recent papers. Before doing this, we checked with the National Science Foundation (specifically, the program director Eva Zanzerkia) that this would not count as collaboration. Therefore, the suggested reviewers do not constitute a conflict of interest. Regarding the JOSS list of reviewers, the following individuals seem qualified to review the paper: Saumik Dana, Tobias Staal, Georgia K Stuart, Matthew Herman, Calum Chamberlain.

Thank you again for the suggestions and comments.

[sbarbot]

There's a lot of MPI code in 3d/src/ratestate.f90 and 3d/src/ratestate_bath.f90, but it seems MPI can't be used for 2D models at all.

The most computationally intensive part of the simulations is Fourier transforms. The one-dimensional discrete Fourier transform has poor performance when parallelized with MPI. This is why MPI is not implemented for the antiplane and plane strain versions of the code. Instead, the parallelization is with OpenMP. The shared memory parallelism offers some improvement up to two threads. In any case, using a fast Fourier transform algorithm makes the calculation much faster than the equivalent calculation in the space domain, by large factors.

Two-dimensional Fourier transforms are simply a series of independent one-dimensional Fourier transforms in one direction followed by a series of one-dimensional Fourier transforms in the other direction. That can be parallelized efficiently. This is why the 3d version of the code implements MPI with significant gains.

[jedbrown]

@mherman09 :wave: Would you be available to review this submission for JOSS?

[mherman09]

Hi Jed,

I would be happy to review. My understanding is that the process is a little different from evaluating a typical scientific manuscript where the code is not the focus, so any guidance would be greatly appreciated.

matt

On Thu, Jan 19, 2023 at 9:35 PM Jed Brown @.***> wrote:

@mherman09 https://github.com/mherman09 👋 Would you be available to review this submission for JOSS?

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

[jedbrown]

Thanks @mherman09, there will be guidance in the review issue (just a moment).

[jedbrown]

@editorialbot add @mherman09 as reviewer

[editorialbot]

@mherman09 added to the reviewers list!

[jedbrown]

@editorialbot add @willic3 as reviewer

[editorialbot]

@willic3 added to the reviewers list!

[jedbrown]

@editorialbot start review

[editorialbot]

OK, I've started the review over in https://github.com/openjournals/joss-reviews/issues/5097.

[willic3]

@editorialbot generate my checklist

[editorialbot]

Checklists can only be created once the review has started in the review issue