pyMBE-dev / pyMBE

pyMBE provides tools to facilitate building up molecules with complex architectures in the Molecular Dynamics software ESPResSo. For an up-to-date API documention please check our website:
https://pymbe-dev.github.io/pyMBE/pyMBE.html
GNU General Public License v3.0
6 stars 8 forks source link

Add automated CI #1

Closed jngrad closed 6 months ago

jngrad commented 6 months ago

Description of changes:

pm-blanco commented 6 months ago

Thank you for taking care of this @jngrad. I would actually try to merge this to main within this week, so I can merge it to the branch were I am working on refactoring the tests for peptides 33-create-folder-paper.

I tried to run locally the current ci test testsuite/LYS_ASP_peptide.py but the path to pyMBE is now broken in the current implementation. I understand that the Github CI ads pyMBE to path when building the job, but it would be convenient to also be able to run the tests locally. What would be the best way to add pyMBE to path, also for the other sample scripts?

jngrad commented 6 months ago

I tried to run locally the current ci test testsuite/LYS_ASP_peptide.py but the path to pyMBE is now broken in the current implementation. I understand that the Github CI ads pyMBE to path when building the job, but it would be convenient to also be able to run the tests locally. What would be the best way to add pyMBE to path, also for the other sample scripts?

The original code used the following logic:

# Find path to pyMBE
current_dir= os.path.dirname(os.path.abspath(inspect.getfile(inspect.currentframe())))
path_end_index=current_dir.find("pyMBE")
pyMBE_path=current_dir[0:path_end_index]+"pyMBE"
sys.path.insert(0, pyMBE_path)

It is quite fragile and imposes restrictions on the naming of folder in which the pyMBE repository gets cloned. Most projects use one of the following techniques:

I think that pyMBE developers would find virtual environments to be the path of least resistance. Virtual environment will become common in Ubuntu 24.04. Here is how to configure a pyMBE-specific virtual environment:

# replace pypresso by python3 in the Makefile
sed -i 's/${ESPResSo_build_path}\/pypresso testsuite/python3 testsuite/' Makefile
# configure a pyMBE-specific virtual environment
python3 -m venv venv
echo 'export OLD_PYTHONPATH="${PYTHONPATH}"' >> venv/bin/activate
echo 'export PYTHONPATH="'$(realpath .)':'$(realpath ../espresso/build/src/python)'${PYTHONPATH:+:$PYTHONPATH}"' >> venv/bin/activate
sed -i '/deactivate () {/a export PYTHONPATH="${OLD_PYTHONPATH}"' venv/bin/activate
source venv/bin/activate
python3 -m pip install -r requirements.txt
python3 -m pip install scipy
deactivate

Notice how you no longer need to hardcode the path to the pypresso script in the Makefile. I don't think you actually need pypresso anywhere, python3 works just fine. Maybe you would need to double-check how run_test_protein.py exposes environment variables to jobs it starts, though. If you truly need pypresso, you could add extra echo and sed expressions to set and reset the $PATH variable.

Every time you want to use pyMBE, you need to update the current terminal environment variables by loading the python virtual environment:

$ echo $PYTHONPATH # empty on my workstation

$ source venv/bin/activate # enter the virtual environment
(venv) $ echo $PYTHONPATH
/work/jgrad/pybme:/work/jgrad/espresso/build/src/python
(venv) $ make testsuite
(venv) $ deactivate # leave the virtual environment
$ echo $PYTHONPATH  # original value should be restored

$
pm-blanco commented 6 months ago

@jngrad I followed your instructions and it works in my laptop, although I still need to run pypresso testsuite/LYS_ASP_peptide.py with pypresso. Otherwise I get:

python3 testsuite/LYS_ASP_peptide.py 
Traceback (most recent call last):
  File "/home/pablo/Escritorio/pyMBE/testsuite/LYS_ASP_peptide.py", line 8, in <module>
    import espressomd
  File "/home/pablo/Escritorio/espresso/build/src/python/espressomd/__init__.py", line 21, in <module>
    from . import _init
ImportError: libboost_serialization.so.1.71.0: cannot open shared object file: No such file or directory

Your proposed solution in a virtual environment sounds good to me, but I have some questions:

and document this both in the paper and the README file. Would this solution make sense to you?

jngrad commented 6 months ago

Your libboost_serialization.so.1.71.0 import error is really weird. The pypresso wrapper script doesn't touch $PATH nor $LD_LIBRARY_PATH. Do you modify $PATH in your .bashrc file? It could also be an issue with the virtual environment: on my workstation, it runs PATH="$VIRTUAL_ENV/bin:$PATH", which may insert the current working directory in the $PATH if $PATH was initially empty. Did you experience this exception on a Linux or macOS laptop? We are planning to write something about virtual environments in the ESPResSo user guide to help with the migration to Ubuntu 24.04, and the issue you are experiencing is something we definitively need to sort out.

I would not recommend putting the virtual environment setup in the Makefile, because that would re-introduce hardcoded paths again. There is no guarantee users put the espresso folder next to the pymbe folder. The Readme file and the user guide would be the right place, because it would force users to copy-paste the bash code and adapt it for their folder tree structure.

Virtual environments are part of the Python standard library since Python 3.3 and are meant to be used without sudo. They work by imitating a Linux installation with local /lib, /bin and /include folders that have precedence over the system-wide folders with the same name; this is quite similar to how modulefiles work, but they are a lot simpler because you can only load one environment (modulefiles load one environment per package, with expensive cross-compatibility checks). They work on compute clusters and with slurm.

pm-blanco commented 6 months ago

@jngrad I am working on Ubuntu 22.04.3 LTS. I did not modify $PATH in my .bashrc file, but it might have to do with some weird bad configuration of the virtual environment, I just now realized that is trying to execute some old espresso compilation that I had on my desktop ~/Escritorio/espresso/build/ instead than the one I actually use ~/espresso4.2/build/pypresso.

As a matter of fact, it looks that the problem that was experiencing has to do with your second point, this old compilation of espresso was indeed just next to my current pyMBE folder on my Desktop! How could we adapt this setup to an user-provided path to pypresso?

Could you please write the guide on how to setup and use pyMBE with this new logic of the virtual environment in the README file and later on the paper in section III-A?

jngrad commented 6 months ago

We could provide a script that users would call with pypresso to configure the environment, e.g.:

pypresso maintainer/setup_venv.py

This would be a lot easier for users who are not familiar with advanced shell syntax. But then it would fall on us to support the other shell interpreters (csh, fish, etc.), which I am not familiar with.

pm-blanco commented 6 months ago

@jngrad we can always provide a general description on how this should be setup and say that, in addition, for users with bash shell interpreters we provide a script maintainer/setup_venv.py to facilitate the setup. Then, we do not need to promise support for the other shell interpreters as we already provide the general guidelines on how to do it and we provide only a supporting tool for the most common case (users with bash shell script). I think that at least in the short run it would be an acceptable solution. What do you think?

pm-blanco commented 6 months ago

@jngrad anyway, we should probably disentangle this issue into two different PR though. Let's focus on the CI for now and we can open a separated PR for the setup and path issue and discuss it with the others. We can probably merge this PR as it is just moving pmb.get_resource to the right place in pyMBE.py (ordered alphabetically) so I can merge it with the refactoring of the tests and the repo structure that I am working on right now.

jngrad commented 6 months ago

I agree, let's avoid unnecessarily delaying this PR. You can run the testsuite locally with:

PYTHONPATH=$(realpath .) make testsuite

The virtual environment can be added in an independent PR.