sntioudis / papreca

PAPRECA hybrid off-lattice kinetic Monte Carlo/molecular dynamics (kMC/MD) simulator
GNU General Public License v2.0
6 stars 0 forks source link

Examples: bad path(s?) #12

Closed liamhuber closed 4 months ago

liamhuber commented 4 months ago

A (very simple but) blocking issue for https://github.com/openjournals/joss-reviews/issues/6714

The papreca_dir="../../build" variable in the Simple Adsorption demo causes the script to fail by not being able to find the Papreca EXE. I believe this is because of the cd, and changing it to papreca_dir="../../../build" worked fine for me.

Similarly, I think the double / in the phosphate example is going to bork things.

sntioudis commented 4 months ago

@liamhuber

Since the PAPRECA build folder can be located in different paths for different users, the 'papreca_dir' variable has to be modified to point to the directory of your PAPRECA executable. This is necessary to run any of the examples (as explained in the relevant sections of the documentation). For instance, this is mentioned in the "Execution section" of the Brownian diffusion example:

Note that, you will have to modify the "papreca_dir" variable in your "test_brownian.sh" script so it points to the path of your PAPRECA executable.

A similar comment was added to the "Execution section" of the Simple adsorption example.

In any case, the "default" 'papreca_dir' value was set to "../../build" because if the user follows the installation instructions in the documentation without changing the paths or the folder names, then running 'test_brownian.sh' from within the Brownian Self Diffusion folder will not fail (since the build folder will be located two subdirectories "up"). If the installation procedure is not followed exactly (e.g., the build folder is not created in the cloned PAPRECA repository folder), then the 'papreca_dir' variable will require modification.

Similarly, I think the double / in the phosphate example is going to bork things.

The double slash is not necessary but will not lead to any issues since consecutive slashes are treated as a single slash in linux bash scripts as stated in the Single Unix specification (version 4), base definitions §3.271 pathname: “Multiple successive slashes are considered to be the same as one slash, except for the case of exactly two leading characters.”

liamhuber commented 4 months ago

Indeed, the double / was a-ok.

I'm not talking about brown Ian but rather simple adsorption where the bash script has a loop and a cd. Here the default path is not compliant with the default build directory. The Brownian one is fine.

sntioudis commented 4 months ago

I'm not talking about brown Ian but rather simple adsorption where the bash script has a loop and a cd.

My bad and thank you for spotting this. I added this line to the 'test_simple_adsdep.sh' script that resolves the real path of PAPRECA to prevent conflicts (in case the 'papreca_dir' variable contains a relative path):

papreca_dir=$(realpath "$papreca_dir")

I just tested this on my workstation and it appears to work fine with a relative papreca path ('papreca_dir=../../build").