In the master branch (2decomp library and pencils domain decomposition), vertically summed slices Zsummed are transposed compared to other slices in the XY plane, ZSlice. For Zsummed slices, the fast index in memory corresponds to x (and y is slow).

This does not happen with the WiP branch (fftw-mpi and slabs domain decomposition), where the output has a consistent memory layout: x is slowest, then y, then z is fastest.

Crash when starting up a coral run due to unallocated variable 'length'. forrtl gives the following error message:
forrtl: severe (408): fort: (8): Attempt to fetch from allocatable variable LENGTH when it is not allocated

Ben,

I noticed that in couple of places you keep setting key variables despite having them already in your environment e.g. CORAL_ROOT in prepare_directory.sh. This happens even if sourcing has occurred before. I suggest you set a default that can be overwritten by the current environment. Check this:

#!/bin/bash
# filename: example.sh
: "${MY_VARIABLE:=my variable}"
echo ${MY_VARIABLE}

and try running with:

./example.sh
MY_VARIABLE ./example.sh

The reason why I think this useful is that first time users have fewer places to edit. There are some benefits of doing what you're doing so I will keep it as a suggestion.

Hi,

When we define an horizontal average as an output in coral.timeseries the vector containing the data always add a 0 as first entry in the array.

Vincent Bouillaut

When running the slab version, the user needs to ensure that he uses a number of processes that divides evenly NX/2 and 3 NY/2, where NX and NY are the number of alias-free modes along x and y, as entered in coral.parameters.in.

As of now, there is no check at runtime. The code simply crashes when ran with an improper number of processes.

It may be more consistent to either use tag of kindOfInformation in both this file and the one describing parameters.

There seems to be a markdown type in the EOF tag given which is given as ``EOF`.

A full definition of a linear variable is called linear_full_variable in this file but linear_variable_full in Input_file_equations most likely indicating that at least one of them is a typo.

Ben,

Can you include in the paper a comparison with other common spectral packages. At the very least I would mention Nek5000 and Nektar++ [2][3].

Most scripts seem to work with both Python3 and Python2. However the dealiase_volumes.py script seems to assume Python2 and produces errors like this when used with Python3 on the First_Run data:

data/plane_layer$ python3 dealiase_volumes.py test1/
[ 001/702 ] :: test1//Volumes/linear_var01_time00000100_full.dat
Traceback (most recent call last):
  File "coral/data/plane_layer/dealiase_volumes.py", line 57, in <module>
    clean_this_dir(directory_to_clean)
  File "coral/data/plane_layer/dealiase_volumes.py", line 52, in clean_this_dir
    dumm_integer = read_and_deAlias(root, my_file)
  File "coral/data/plane_layer/dealiase_volumes.py", line 28, in read_and_deAlias
    aux2 = np.delete(aux1, np.arange(NXAA/3+1,2*NXAA/3+1) , axis = 1)
  File "<__array_function__ internals>", line 5, in delete
  File "/usr/lib/python3/dist-packages/numpy/lib/function_base.py", line 4406, in delete
    keep[obj,] = False
IndexError: arrays used as indices must be of integer (or boolean) type

This dependency on a particular (also no longer supported) Python version should be documented, yet I cannot find it mentioned in the wiki or documentation.

The first run and benchmark examples provide a way to test that one's installation of Coral works as expected, yet they only provide graphs requiring that one eyeball compares one's results with the image files in the documentation eg https://github.com/BenMql/coral/wiki/benchmark01_rayleigh_benard

For openjournals/joss-reviews#2978 it would be good if some of the data files of the benchmark runs were also provided so that a more quantitative comparison (even if only plotting both datasets on the same graph) was possible.

Hello Ben,

I think there are still issues with the make file. When I run make pencils I run into this error:

mpif90 -cpp -O2 -march=native -c /home/rsa/documents/reviews/to_review/miquel2021//coral/src/sparse_tools/sparse_manipulations.f90 -o /home/rsa/documents/reviews/to_review/miquel2021//coral/src/sparse_tools/sparse_manipulations.o -J/home/rsa/documents/reviews/to_review/miquel2021//coral/.mod/ -I/home/rsa/documents/reviews/to_review/miquel2021/2decomp_emptyFourierFourier//include
/home/rsa/documents/reviews/to_review/miquel2021//coral/src/sparse_tools/sparse_manipulations.f90:16:6:

   16 |  use fortran_kinds
      |      1
Fatal Error: Cannot open module file ‘fortran_kinds.mod’ for reading at (1): No such file or directory
compilation terminated.
make: *** [makefile:42: /home/rsa/documents/reviews/to_review/miquel2021//coral/src/sparse_tools/sparse_manipulations.o] Error 1

fortran_kinds.f90 sits in MISC_DIR but for some reason the module file is not generated before (is it meant to?). Perhaps something is out of order in the makefile?

After fixing this I am progressing with the remaining instructions. I create an environment file, modify accordingly and then the build crashes immediately.

My overall feedback is that some instructions are actually too long. For instance the whole discussion about environment files can be much abbreviated. You could try an enumerated list and simple imperative statements.

You can highlight important variables as sub-bullets in "modify the template" step.

As of v1.1.5, exporting horizontally-averaged profiles as functions of height z yields rubbish.
It is best to avoid >>Profiles in coral.usrOutput until this is fixed.

Exporting slices of the variables is disabled since commit 55fc825

The file Input_usrOutput seems incorrectly named and the wiki page on input files 5_Input_files actually tries to refer to it as Input_file_usrOutput which is consistent with the other input file wiki pages but does not match the actual file name.

Most of the comments in #14 also apply to this file.

The time index resets to zero when restarting a run. This means filenames generated using the time index will match those from previous runs, despite the simulation being further along in nondimensional time. This causes saved profiles, slices, volumes, etc. to be overwritten with the latest results.

I suggest adding topics such as partial-differential-equations,pde in the About section.

In 4_data_visualisation one is asked to visualize xy averaged data like so:

python plot_xyAvg_timeseries.py tFull 23 tFull 35

yet running command yields

plane_layer/test1$ python3 plot_xyAvg_timeseries.py tFull 23 tFull 35
Traceback (most recent call last):
  File "coral/data/plane_layer/test1/plot_xyAvg_timeseries.py", line 8, in <module>
    avgQty = np.fromfile('./Timeseries/'+argv[iarg+1] +'.bin', dtype=np.float_)
FileNotFoundError: [Errno 2] No such file or directory: './Timeseries/tFull.bin'

and the Timeseries directory contains only

plane_layer/test1$ ls Timeseries/
tFull_XYavg_z00023.dat  time.dat       uu_volAvg.dat  ww_volAvg.dat
tFull_XYavg_z00035.dat  tw_volAvg.dat  vv_volAvg.dat

for me.

This

diff --git a/etc/python_scripts/plot_xyAvg_timeseries.py b/etc/python_scripts/plot_xyAvg_timeseries.py
index 49f13b6..ab2fca5 100644
--- a/etc/python_scripts/plot_xyAvg_timeseries.py
+++ b/etc/python_scripts/plot_xyAvg_timeseries.py
@@ -4,8 +4,8 @@ from sys import argv

 plt.figure()
 time = np.fromfile('./Timeseries/time.dat', dtype=np.float_)
-for iarg in range(len(argv)-1):
-    avgQty = np.fromfile('./Timeseries/'+argv[iarg+1] +'.bin', dtype=np.float_)
+for iarg in range(0,len(argv)-1,2):
+    avgQty = np.fromfile('./Timeseries/'+argv[iarg+1]+'_XYavg_'+'z%05d'%int(argv[iarg+2])+'.dat', dtype=np.float_)
     plt.plot(time, avgQty [: time.shape[0]])
     del avgQty

makes the plot work

In GAFD flows, it would be useful to decompose variables into a barotropic (vertically invariant) and baroclinic (zero vertical mean) components, at least for diagnostics (i.e. when variables are computed in physical space).

When defining a >>linear_variable_full object in coral.equations, any vertical derivative of a (horizontally averaged) mean variable is ignored. This will be implemented soon as this is useful for defining the components of the vorticity (exemplified below using a formulation based on velocity potentials and mean flows):

>>====================================<<
>>linear_variable_kxky :: psi <<
>>linear_variable_kxky :: phi <<
>>linear_variable_mean :: uMean <<
>>linear_variable_mean :: vMean <<
>>====================================<<
>>linear_variable_full       :: uFull <<
>>linear_variable_full_build :: + uMean <<
>>linear_variable_full_build :: + dy.psi <<
>>linear_variable_full_build :: + dx.dz.phi <<
>>====================================<<
>>linear_variable_full       :: vFull <<
>>linear_variable_full_build :: + vMean <<
>>linear_variable_full_build :: - dx.psi <<
>>linear_variable_full_build :: + dz.dy.phi <<
>>====================================<<
>>linear_variable_full       :: wFull <<
>>linear_variable_full_build :: - dx.dx.phi <<
>>linear_variable_full_build :: - dy.dy.phi <<
>>====================================<<
>>linear_variable_full       :: vortX <<
>>linear_variable_full_build :: - dy.dx.dx.phi <<
>>linear_variable_full_build :: - dy.dy.dy.phi <<
>>linear_variable_full_build :: + dz.dx.psi <<
>>linear_variable_full_build :: - dz.dz.dy.phi <<
>>linear_variable_full_build :: - dz.vMean <<
>>====================================<<

As of v1.1, the code will ignore dz in the last line above.

Hi Benjamin,
I cloned the recent version of the code and tried to compile it on the cluster.
I'm getting the below errors-

/analysis/subhajitkar/data/Results/coral/src/pencils.2decomp/domain_decomposition.f90(18): error #7002: Error in opening the compiled module file. Check INCLUDE paths. [DECOMP_2D]
use decomp_2d
-------^
/analysis/subhajitkar/data/Results/coral/src/pencils.2decomp/domain_decomposition.f90(19): error #7002: Error in opening the compiled module file. Check INCLUDE paths. [DECOMP_2D_FFT]
use decomp_2d_fft
-------^
/analysis/subhajitkar/data/Results/coral/src/pencils.2decomp/domain_decomposition.f90(64): error #6404: This name does not have a type, and must have an explicit type. [ZSTART]
self%phys_istart = zstart !< copy 2decomp's internal var.
-------------------------^
/analysis/subhajitkar/data/Results/coral/src/pencils.2decomp/domain_decomposition.f90(65): error #6404: This name does not have a type, and must have an explicit type. [ZEND]
self%phys_iEnd = zend !< copy 2decomp's internal var.
-------------------------^
/analysis/subhajitkar/data/Results/coral/src/pencils.2decomp/domain_decomposition.f90(66): error #6404: This name does not have a type, and must have an explicit type. [ZSIZE]
self%phys_iSize = zsize !< copy 2decomp's internal var.
-------------------------^
/analysis/subhajitkar/data/Results/coral/src/pencils.2decomp/domain_decomposition.f90(67): error #6404: This name does not have a type, and must have an explicit type. [IN_CORE_CHEBYSHEV]
call decomp_2d_fft_init(IN_CORE_CHEBYSHEV)
------------------------------^

Do you know how I am getting these errors?
Any help would be very much appreciated!

Thank you,
Subhajit

Hi Ben,

I forked your wiki page and fixed minor typos in your documentation. It was all written up very well, but there were a few consistent mistakes. For instance time step and time series are still spelled separately anywhere outside of the command line 😉. I also capitalised "Intel", "Fortran" etc.

Did you know you can edit your wiki locally with any editor? Simply add .wiki.git e.g.

git clone https://github.com/BenMql/coral.wiki.git

I typically use VIM with a spellchecker, find and replace and other useful functionalities. You might be using something else, but I think it's useful to know that wiki is also a Git repo.

If you want then to use the wiki I forked and corrected you need to do something like this

git clone https://github.com/BenMql/coral.wiki.git
cd coral.wiki
git remote add rms https://github.com/robertsawko/coral.wiki.git
git fetch rms
git merge rms/master

Your mileage may vary! It's a bit clumsy, but I think GH still does not allow to do a PR for Wikis. What a wonderful world that would have been. 😉

Ben,

Can you also expand the statement of need in the docs here. The problem the code solves is compactly stated, but the checklist is requesting to include a target audience as well. Note that you mention target audience in the paper.

Also note two minor typos:

• eventhough is spelled separately even though,
• GitHub markdown requires double star to make the text bold, which I think was your intention with the acronym.

The notebook refers to linear_variable_zero which it seems to me ought to be linear_variable_mean, at least no _zero variable has been encountered by my reading of the documtenation so far.

Entering a lonely term (as opposed to a combination of contributions separated by dots) in coral.equations is not interpreted properly.

For instance:

>>add_rhs_source :: + source04 <<

yields trouble. For now, the following workaround should be used:

>>Param :: one := 1.d0 <<
>>add_rhs_source :: + one.source04 <<

21-04-2022 Edit: This issue impacts only sources term >>add_rhs_source. Other contributions work as intended.

I seem to be unable to find guidelines for potential contributors. Both technical (make a pull request, create a ticket with a patch, quality expectations, any required formatting style of code) and non-technical (is any transfer of copyright expected?).

related to: openjournals/joss-reviews#2978

Hi Ben,

I started reviewing your JOSS submission today. Very interesting piece of research and congratulations on your results and publications so far.

Minor correction. As per bot suggestion can you add the DOI to the references for your APS paper:

10.1103/physrevfluids.4.121501

Ben,

Consider setting up the variables in the instructions. It's more compact than writing a sentence on the variable meaning. Novice users will be able to copy-paste and experienced users will interpret it immediately and adjust for their need e.g.

git clone ...
cd coral
CORAL_ROOT=$(pwd)
# or export CORAL_ROOT=$(PWD)

Same for 2decomp&fft instructions.

When solving equations with sources, coral crashes at runtime because some buffer allocatable do not have the right size when using a slab domain decomposition

In the plane_layer_volume class, the xdiff and ydiff methods that compute derivatives along the horizontal coordinates yield an inexact result.

In Input_file_equations the wiki refers to a file LP_user_sources_definition.f90 which is incorrect since the file is named LP_user_sources_definitions.f90 with an s.

The >>EOF<< marker is only mentioned in the Pittfalls section.

There's also some (English language typos) and an inconsistency in that the directory called coral in that file is designated using the $CORAL_ROOT variable in other places.

Hi,

The function zint() of the plane_layer_volume package creates a profile + a verticalSums even when applied on a profile instead of only a verticalSums.

Vincent Bouillaut

Firstly, I was hit by a missing trailing slash for the directory. You may want to add / after $(CORAL_ROOT) in makefile. There may be a more robust way of doing this to avoid double slashes (pathJoin in Lmod), but I would assume it's better to have a double slash than a missing one as one results in an error and the other one doesn't.

I am stuck though on the -module option.

mpif90 -cpp -O2 -march=native -c /opt/intel/mkl/include/lapack.f90 -o /home/rsa/documents/reviews/to_review/miquel2021//coral/src/cheby_tools/lapack_module.o -M/home/rsa/documents/reviews/to_review/miquel2021//coral/.mod/
gfortran: error: unrecognized command-line option ‘-M/home/rsa/documents/reviews/to_review/miquel2021//coral/.mod/’
make: *** [makefile:28: /home/rsa/documents/reviews/to_review/miquel2021//coral/src/cheby_tools/lapack_module.o] Error 1