JOSS REVIEW: Installation Instructions

[rbeucher]

Hi All,

I am starting my review for JOSS.

First of all. Thanks for submitting the code. The submission is in great shape. Hopefully it won't take too long... Sorry I'm starting a bit late. It's the beginning of the academic year here in Australia.

A few remarks regarding the installation instructions.

• [ ] Please specify the latest version of PETSc that you support (PETSc has a tendency to introduce breaking changes...) I would pin the version for the JOSS submission.
• [ ] You are relying on PETSc configure to get the external package which is the right thing to do. Maybe you should tell the user that they will have to update a few environment variables (PETSC_DIR, PATH (for mpicc, mpirun))
• [ ] Maybe list build requirements and running requirements?
• [ ] Are all PETSc external packages required or some of them are optional?
• [ ] I suppose the code will eventually be used on HPC. Might be good to add some information on the requirements (COMPILERS, BLAS (openblas?, MKL?), parallel HDF5?, MPICH vs OPENMPI?, CMAKE version (although I suppose this one is for building SUPERLU_DIST)
• [ ] Why is gfortran optional?
• [ ] A Docker container would be greatly appreciated...

:warning: In the Mandyoc Makefile, I would not default to $HOME/petsc you should specifically ask for the PETSC_DIR to be set by the user. That will save you lots of trouble...

[psanan]

A couple of additional comments on the installation instructions:

• What does the optional dependency "os" mean? There is the Python os module, but that's part of the standard library so is present whenever Python is, and this isn't an easy question for me to ask a search engine 😃 Maybe just "Python" should be listed here?
• Consider adding a more complete set of Python packages, beyond NumPy. If I run e.g. git grep import I see various scripts requiring Matplotlib and Pandas.

[rafaelmds]

A Docker container would be greatly appreciated...

We now provide a docker image (merged in #57).

[rafaelmds]

Please specify the latest version of PETSc that you support (PETSc has a tendency to introduce breaking changes...)
  I would pin the version for the JOSS submission.

We added this information in installation instructions. Currently tested petsc version is v3.15.5 (#69)

You are relying on PETSc configure to get the external package which is the right thing to do. Maybe you should tell the user that they will have to update a few environment variables (PETSC_DIR, PATH (for mpicc, mpirun))

This was covered by #55. Also, we refer to theses variables in How to instal section and the new examples in jupyter notebooks.

Maybe list build requirements and running requirements?
Are all PETSc external packages required or some of them are optional?

Done. We improved installation instructions (#69). We also explain that some external packages are necessary for MUMPS, which is used by default to provide direct solvers.

I suppose the code will eventually be used on HPC. Might be good to add some information on the requirements (COMPILERS, BLAS (openblas?, MKL?), parallel HDF5?, MPICH vs OPENMPI?, CMAKE version (although I suppose this one is for building SUPERLU_DIST)

Mandyoc is intended to be used on HPC. Parallel communication is done by MPI via PETSc. We recognize that we don't know advantages in use of MPICH vs OPENMPI and other libraries that can be swapped using PETSc.

SUPERLU_DIST and hence, cmake, was removed from PETSc example installation since it is not required.

Why is `gfortran` optional?

We simplified the installation instructions to require gfortran for PETSc installation. But we point the user to PETSc docs to see available options for installation. We phrase that the instructions are just a minimal installation instruction necessary to run Mandyoc.

warning In the Mandyoc Makefile, I would not default to $HOME/petsc you should specifically ask for the PETSC_DIR to be set by the user. That will save you lots of trouble...

We hope we solved this in #55.

[psanan]

Minor points on the installation instructions:

• gfortran is stated as a dependency for PETSc. Most Fortran compilers (not just that one) will work (--with-fc=myfortrancompiler) and it is also possible to configure PETSc without Fortran by using --with-fc=0. The wrinkle in that case is that if you were using --download-fblaslapack, you need to change it to --download-f2cblaslapack)
• Typo: "extrictly needded" --> "strictly needed"
• This sentence has formatting problems: "Finally, add a symlinks of mpirun to ~/.local/bin" (and a typo: "symlinks" --> "symlink")

[rafaelmds]

@psanan Thanks for the review.

Yes, in fact in the most cases there is no need to use --with- = compiler... options.