[PRE REVIEW]: Mandyoc: A finite element code to simulate thermochemical convection in parallel #3551

whedon · 2021-07-29T22:51:21Z

Submitting author: @victorsacek (Victor Sacek)
Repository: https://github.com/ggciag/mandyoc
Version: v0.1.1
Editor: @jedbrown
Reviewers: @rbeucher
Managing EiC: Kyle Niemeyer

⚠️ JOSS reduced service mode ⚠️

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 badge code:

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

Author instructions

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

@victorsacek 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 @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands

The text was updated successfully, but these errors were encountered:

whedon · 2021-07-29T22:51:24Z

Hello human, I'm @whedon, a robot that can help you with some common editorial tasks.

⚠️ JOSS reduced service mode ⚠️

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.

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 · 2021-07-29T22:51:32Z

Wordcount for paper.md is 817

whedon · 2021-07-29T22:51:34Z

Software report (experimental):

github.com/AlDanial/cloc v 1.88  T=0.12 s (490.0 files/s, 96813.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C++                             14           2303            350           5185
Python                          13            371            316            718
TeX                              2             34              0            342
Markdown                         6             82              0            248
reStructuredText                 9            343            397            247
C/C++ Header                     1             91             10            244
make                             2             16              4             64
YAML                             1              9             21             35
Bourne Shell                     9              1              2             16
CSS                              1              1              0              9
-------------------------------------------------------------------------------
SUM:                            58           3251           1100           7108
-------------------------------------------------------------------------------


Statistical information for the repository '76ee3286be0dee889ffe98a5' was
gathered on 2021/07/29.
The following historical commit information, by author, was found:

Author                     Commits    Insertions      Deletions    % of changes
Agustina                         1             1              1            0.01
Agustina Pesce                   1            17             10            0.20
Dave May                         1             2              0            0.01
Jamison Assunção                 3           716            700           10.40
Rafael Monteiro da S             3           391              4            2.90
Rafael Silva                     4           907             11            6.75
Victor Sacek                    69          9325           1524           79.72

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
Agustina                     18         1800.0          1.4                0.00
Dave May                      2          100.0         16.0                0.00
Jamison Assunção            701           97.9          0.9               22.11
Rafael Monteiro da S       1175          300.5          6.3                4.60
Victor Sacek               7692           82.5          3.4                3.95

whedon · 2021-07-29T22:51:41Z

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

OK DOIs

- 10.1051/0004-6361/201629272 is OK
- 10.1051/0004-6361/201322068 is OK

MISSING DOIs

- 10.1111/j.1365-246x.2012.05388.x may be a valid DOI for title: A comparison of numerical surface topography calculations in geodynamic modelling: an evaluation of the ‘sticky air’method
- 10.1016/j.pepi.2010.04.007 may be a valid DOI for title: A stabilization algorithm for geodynamic numerical simulations with a free surface
- 10.1007/978-1-4612-1986-6_8 may be a valid DOI for title: Efficient Management of Parallelism in Object Oriented Numerical Software Libraries
- 10.1016/j.epsl.2016.11.026 may be a valid DOI for title: Post-rift influence of small-scale convection on the landscape evolution at divergent continental margins
- 10.1016/j.jog.2021.101830 may be a valid DOI for title: Lateral flow of thick continental lithospheric mantle during tectonic quiescence

INVALID DOIs

- None

whedon · 2021-07-29T22:51:52Z

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

kyleniemeyer · 2021-07-29T22:54:30Z

@victorsacek thanks for your interest in JOSS! Our editors in this area are at capacity right now, so I'm going to place this on our waitlist until someone becomes available.

kyleniemeyer · 2021-07-29T22:54:32Z

@jedbrown I think you are at editing capacity right now, but wanted to give you a heads-up: this looks to be right in your wheelhouse.

jedbrown · 2021-08-06T05:30:55Z

@whedon assign @jedbrown as editor

whedon · 2021-08-06T05:31:00Z

OK, the editor is @jedbrown

jedbrown · 2021-08-18T04:09:40Z

Thanks for this submission. While I locate reviewers, could I ask that you update the project documentation to avoid the many places where developer paths are hard-coded (and thus won't work for users). I think most of these can be removed rather than "fixed", and the documentation can state how to run the code without these scripts. (You could audit these steps in a container or a third-party computer to ensure the instructions make sense and actually work.)

$ git grep /Users
examples/continental_rift/run.sh:/Users/victorsacek/Documents/petsc/arch-label-debug/bin/mpirun -n 8 \
examples/continental_rift/run.sh:/Users/victorsacek/Documents/gits/md2d/md2d_aux/mandyoc \
examples/old_version/Crameri2012_Case2/run.sh:nohup /Users/victorsacek/Documents/petsc/arch-label-debug/bin/mpirun -n 4 ./mandyoc -denok 1.0e-15 -rtol 1.0e-7 -particles_per_ele 1000 -theta_FSSA 0.5 -sub_division_time_step 1.0 -visc_harmonic_mean 0 -particles_perturb_factor 0 -visc_const_per_element 1 -pressure_in_rheol 1 <FD.in >FD.out &
examples/old_version/vanKeken1997/run.sh:nohup /Users/victorsacek/Documents/petsc/arch-label-debug/bin/mpirun -n 4 ./mandyoc -denok 1.0e-15 -particles_per_ele 1000 -theta_FSSA 0.5 -sub_division_time_step 1.0 -visc_harmonic_mean 0 -particles_perturb_factor 0 -visc_const_per_element 1 -pressure_in_rheol 1 <FD.in >FD.out &
examples/old_version/vanKeken1997_nondim/run-mg.sh:nohup /Users/victorsacek/Documents/petsc/arch-label-debug/bin/mpirun -n 4 ./mandyoc -denok 1.0e-15 -particles_per_ele 1000 -theta_FSSA 0.5 -sub_division_time_step 1.0 -visc_harmonic_mean 0 -particles_perturb_factor 0 -visc_const_per_element 1 -direct_solver 0 -pressure_in_rheol 1 -veloc_ksp_monitor_true_residual -veloc_ksp_type fgmres -veloc_pc_type mg -veloc_pc_mg_galerkin -veloc_pc_mg_levels 3 -veloc_mg_levels_ksp_max_it 8 <FD.in >FD.out &
examples/old_version/vanKeken1997_nondim/run.sh:nohup /Users/victorsacek/Documents/petsc/arch-label-debug/bin/mpirun -n 4 ./mandyoc -denok 1.0e-15 -particles_per_ele 1000 -theta_FSSA 0.5 -sub_division_time_step 1.0 -visc_harmonic_mean 0 -particles_perturb_factor 0 -visc_const_per_element 1 -direct_solver 0 -pressure_in_rheol 1 <FD.in >FD.out &
examples/vanKeken1997_case1a/run.sh:/Users/kugelblitz/opt/petsc/arch-0-fast/bin/mpirun -n 2 /Users/kugelblitz/Desktop/mandyoc-misc/mandyoc/mandyoc
src/compile.sh:make all PETSC_DIR=/Users/victorsacek/Documents/petsc PETSC_ARCH=arch-label-debug
src/compile_opt.sh:make all PETSC_DIR=/Users/victorsacek/Documents/petsc PETSC_ARCH=arch-label-optimized

In the paper, please also explain its positioning relative to other software in this space. You don't have to be comprehensive, but please communicate enough about why a researcher would choose this over a representative set of well-known packages.

The equations can be formatted with \begin{align} or \begin{gather} rather than a sequence of display maths.

I see the acknowledgement mentions multigrid, but the van Keken benchmark is configured to use MUMPS. A note on solvers would be useful (even if the intent is that expert users tune using PETSc options). The paper also doesn't mention whether it supports 3D problems and at what scale it has been used. The code isn't entirely helpful in that "future 3d version" appears in comments, while some functions say they are 3d.

Do you have automated tests? I see you have a GitHub action to build the docs, but not to test correctness. Some automated tests should be provided.

Have you used a code coverage tool to see if there is significant dead code?

Thank you for the pleasant build experience. There were a few -Wunused-but-set-variable warnings that may as well be fixed.

aguspesce · 2021-08-23T14:10:00Z

Hello @jedbrown,
Thank you for taking your time to make the review!
We are working to fix and improve the observation that you mentioned.

jedbrown · 2021-08-23T15:42:00Z

@aguspesce Great! Just let reply here after making updates. You can comment @whedon generate pdf to spin a new PDF at that time.

aguspesce · 2021-09-27T13:39:54Z

@whedon generate pdf

whedon · 2021-09-27T13:40:43Z

👉📄 Download article proof 📄 View article proof on GitHub 📄 👈

victorsacek · 2021-09-27T14:04:15Z

Hello @jedbrown,

Thank you very much for the review of our code.

We've made almost all the changes that you suggested:

Improved the examples:
- Fixed path.
- Added README file with running instructions.
- Improved examples script to create input files and plot results.
Made test to check if Mandyoc's output result for a given model is equal to the expected result
Improved paper
Added developers info
Fixed -Wunused-but-set-variable warnings during Mandyoc compilation
Fixed documentation
Improved Github Actions for documentation

We have to clarify that we decided not to create an automated test using GitHub actions because installing all dependencies to run Mandyoc and test it would take too much time for the CI.
But we created a test based on regression testing.
This test can be executed once the program is installed.
We intend to improve the code to use Docker in the future.

All the changes are in the release v0.1.2.

Please inform us if the present version of the code is ok for review in JOSS.

Thank you for your attention.

@aguspesce @jamisonassuncao @rafaelmds

aguspesce · 2021-11-29T19:10:40Z

Hello @jedbrown, is there any update on the pre review process of this submission?

cc @danielskatz

aguspesce · 2022-01-07T14:32:47Z

Hello @jedbrown, is there any update on the pre review process of this submission?

cc @danielskatz

jedbrown · 2022-01-07T17:17:44Z

@aguspesce Yikes, I'm terribly sorry about my silence. Thanks for your effort here; it looks good. I see CONTRIBUTING.md says to run make test, but it actually needs make test_mandyoc (or change the Makefile target). The scripts also look inconsistent in where they get mpirun (PETSc directory or $MPI_PATH). It's probably best to make this a run-time option, but you have some choices

make getmpiexec with your Makefile
$(MPIEXEC) inside your Makefile
$(dirname $(pkg-config --variable=ccompiler $PETSC_DIR/$PETSC_ARCH/lib/pkgconfig/petsc.pc))/mpiexec (less reliable)

Here's a patch to make the test run out of the box, but the scripts still use the fragile style.

diff --git i/Makefile w/Makefile
index e0e91de..3d2f3e6 100644
--- i/Makefile
+++ w/Makefile
@@ -26,8 +26,6 @@ SOURCEC = $(SRC)/main.cpp \
 	$(SRC)/sp.cpp
 OBJECTS = $(SOURCEC:%.cpp=%.o)
 
-MPI_PATH = ${PETSC_DIR}/${PETSC_ARCH}/bin
-
 help:
 	@echo ""
 	@echo "Commands:"
@@ -40,7 +38,7 @@ help:
 test_mandyoc:
 
 	@echo "Run MANDYOC test may take several minutes.."
-	cd test/testing_data/ ; ${MPI_PATH}/mpirun -n 2 ../../mandyoc
+	cd test/testing_data/ ; ${MPIEXEC} -n 2 ../../mandyoc
 	pytest -v test/testing_result.py 
 
 # Build Mandyoc

I ran this and get one test failure (0-velocity):


field = 'velocity', step = 0

    @pytest.mark.parametrize("field", fields)
    @pytest.mark.parametrize("step", steps)
    def test_result(field, step):
        """ """
        filename = f"{field}_{step}" + ".txt"
        output = read(test_path / filename)
        expected = read(expected_path / filename)

>       npt.assert_allclose(output, expected, rtol=1e-5)
E       AssertionError:
E       Not equal to tolerance rtol=1e-05, atol=0
E
E       Mismatched elements: 1 / 94978 (0.00105%)
E       Max absolute difference: 1.e-16
E       Max relative difference: 1.10695082e-05
E        x: array([0., 0., 0., ..., 0., 0., 0.])
E        y: array([0., 0., 0., ..., 0., 0., 0.])

test/testing_result.py:57: AssertionError

I'll find reviewers.

jedbrown · 2022-01-07T17:29:22Z

@victorsacek @aguspesce Could you please include in your statement of need some related packages to help a prospective user understand the relative merits of mandyoc versus other software packages in this domain?

jedbrown · 2022-01-07T17:29:48Z

@gassmoeller @rbeucher 👋 Would you be available to review this submission to JOSS?

victorsacek · 2022-01-12T12:40:59Z

Hello @jedbrown

Thank you very much for your attention.

We modified the Makefile, changing the MPI variables for the test, as you suggested.

Additionally, in the Statement of need, we added one sentence explaining that one advantage of Mandyoc is the possibility to incorporate variable boundary conditions in space and time, appropriate to simulate different pulses of tectonism in one model run.

About the failure test, we tried different machines but we couldn't reproduce this failure. Please, if possible, could you send us more details about the problem?

Best regards

rbeucher · 2022-01-12T21:45:55Z

Hi @jedbrown,

Yes I can review this submission. Looks interesting.

R

gassmoeller · 2022-01-13T22:20:41Z

Hi @jedbrown thanks for asking. This would normally be right in my area of interest, but I am pretty tied up at the moment and would not get to the review before mid March. Maybe @bangerth or @tjhei are available?

jedbrown · 2022-01-15T05:02:10Z

@victorsacek Thanks for your updates. I think you'll find that other software supports BCs that depend on space and time. I think this section should be specific in placing present work in the context of related work. In this case, I think it involves citing some alternatives and explaining the relative merits.

Also, in the Makefile, it's better not to change MPIEXEC, which has already been defined in the makefile you included. Your redefinition prevents use of system MPI (i.e., when not using --download-mpich or --download-openmpi).

Thanks for being willing to review, @rbeucher! I'll add you and start the review. I have some emails out and will find a second reviewer as well. Of course I'd be thrilled if @bangerth or @tjhei are available. And I totally understand this present state of being burried @gassmoeller; thanks for your reply.

jedbrown · 2022-01-15T05:02:39Z

@whedon add @rbeucher as reviewer

whedon · 2022-01-15T05:02:44Z

OK, @rbeucher is now a reviewer

jedbrown · 2022-01-15T05:02:55Z

@whedon start review

whedon · 2022-01-15T05:02:59Z

OK, I've started the review over in #4070.

rafaelmds · 2022-01-18T13:22:27Z

@whedon check references

whedon · 2022-01-18T13:22:38Z

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

OK DOIs

- 10.1111/j.1365-246X.2012.05388.x is OK
- 10.1016/j.pepi.2010.04.007 is OK
- 10.1007/978-1-4612-1986-6_8 is OK
- 10.1016/j.epsl.2016.11.026 is OK
- 10.1016/j.jog.2021.101830 is OK
- 10.1029/97JB01353 is OK

MISSING DOIs

- None

INVALID DOIs

- None

whedon added the pre-review label Jul 29, 2021

whedon added C++ Makefile TeX labels Jul 29, 2021

kyleniemeyer added the waitlisted Submissions in the JOSS backlog due to reduced service mode. label Jul 29, 2021

whedon assigned jedbrown Aug 6, 2021

danielskatz removed the waitlisted Submissions in the JOSS backlog due to reduced service mode. label Sep 28, 2021

aguspesce mentioned this issue Jan 11, 2022

Improve Makefile changing the MPI variable for the test ggciag/mandyoc#39

Merged

whedon assigned jedbrown and rbeucher and unassigned jedbrown Jan 15, 2022

whedon closed this as completed Jan 15, 2022

This was referenced Jan 18, 2022

Missing DOIs ggciag/mandyoc#40

Closed

Revert and fix MPIEXEC in Makefile ggciag/mandyoc#41

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PRE REVIEW]: Mandyoc: A finite element code to simulate thermochemical convection in parallel #3551

[PRE REVIEW]: Mandyoc: A finite element code to simulate thermochemical convection in parallel #3551

whedon commented Jul 29, 2021 •

edited

Loading

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

kyleniemeyer commented Jul 29, 2021

kyleniemeyer commented Jul 29, 2021

jedbrown commented Aug 6, 2021

whedon commented Aug 6, 2021

jedbrown commented Aug 18, 2021

aguspesce commented Aug 23, 2021

jedbrown commented Aug 23, 2021

aguspesce commented Sep 27, 2021

whedon commented Sep 27, 2021

victorsacek commented Sep 27, 2021 •

edited

Loading

aguspesce commented Nov 29, 2021

aguspesce commented Jan 7, 2022

jedbrown commented Jan 7, 2022

jedbrown commented Jan 7, 2022

jedbrown commented Jan 7, 2022

victorsacek commented Jan 12, 2022

rbeucher commented Jan 12, 2022

gassmoeller commented Jan 13, 2022

jedbrown commented Jan 15, 2022

jedbrown commented Jan 15, 2022

whedon commented Jan 15, 2022

jedbrown commented Jan 15, 2022

whedon commented Jan 15, 2022

rafaelmds commented Jan 18, 2022

whedon commented Jan 18, 2022

[PRE REVIEW]: Mandyoc: A finite element code to simulate thermochemical convection in parallel #3551

[PRE REVIEW]: Mandyoc: A finite element code to simulate thermochemical convection in parallel #3551

Comments

whedon commented Jul 29, 2021 • edited Loading

Status

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

whedon commented Jul 29, 2021

kyleniemeyer commented Jul 29, 2021

kyleniemeyer commented Jul 29, 2021

jedbrown commented Aug 6, 2021

whedon commented Aug 6, 2021

jedbrown commented Aug 18, 2021

aguspesce commented Aug 23, 2021

jedbrown commented Aug 23, 2021

aguspesce commented Sep 27, 2021

whedon commented Sep 27, 2021

victorsacek commented Sep 27, 2021 • edited Loading

aguspesce commented Nov 29, 2021

aguspesce commented Jan 7, 2022

jedbrown commented Jan 7, 2022

jedbrown commented Jan 7, 2022

jedbrown commented Jan 7, 2022

victorsacek commented Jan 12, 2022

rbeucher commented Jan 12, 2022

gassmoeller commented Jan 13, 2022

jedbrown commented Jan 15, 2022

jedbrown commented Jan 15, 2022

whedon commented Jan 15, 2022

jedbrown commented Jan 15, 2022

whedon commented Jan 15, 2022

rafaelmds commented Jan 18, 2022

whedon commented Jan 18, 2022

whedon commented Jul 29, 2021 •

edited

Loading

victorsacek commented Sep 27, 2021 •

edited

Loading