[JOSS REVIEW] Support for Python>3.6

[ml-evs]

Hi there, I'll be reviewing this repo for JOSS over at https://github.com/openjournals/joss-reviews/issues/5035.

I'll raise a few issues in this repo as I go -- I'll try to keep issues in this repo focused on actionable code changes, and will instead provide paper feedback etc. in the main review thread, so please keep an eye on both.

First up, I am trying to install the package with conda (22.11) following the instructions in the README. For the env_cpu.yml, conda hangs deep in the dependency solver. Switching to mamba seems to work fine (it uses a different dependency solver), and the following environment is created:

env_cpu_mamba.yml:

``` name: amptorch_mamba channels: - pytorch - conda-forge - defaults dependencies: - _libgcc_mutex=0.1=conda_forge - _openmp_mutex=4.5=2_gnu - alsa-lib=1.2.7.2=h166bdaf_0 - appdirs=1.4.4=pyh9f0ad1d_0 - ase=3.21.1=pyhd8ed1ab_0 - blas=1.0=mkl - bzip2=1.0.8=h7f98852_4 - c-ares=1.18.1=h7f98852_0 - ca-certificates=2022.12.7=ha878542_0 - cached-property=1.5.2=hd8ed1ab_1 - cached_property=1.5.2=pyha770c72_1 - certifi=2021.5.30=py36h5fab9bb_0 - cfgv=3.3.1=pyhd8ed1ab_0 - click=8.0.1=py36h5fab9bb_0 - cpuonly=2.0=0 - cudatoolkit=11.1.1=ha002fc5_11 - cycler=0.11.0=pyhd8ed1ab_0 - dataclasses=0.8=pyh787bdff_2 - dbus=1.13.6=h5008d03_3 - distlib=0.3.6=pyhd8ed1ab_0 - editdistance-s=1.0.0=py36h605e78d_1 - expat=2.5.0=h27087fc_0 - filelock=3.4.1=pyhd8ed1ab_0 - flask=2.0.3=pyhd8ed1ab_0 - font-ttf-dejavu-sans-mono=2.37=hab24e00_0 - font-ttf-inconsolata=3.000=h77eed37_0 - font-ttf-source-code-pro=2.038=h77eed37_0 - font-ttf-ubuntu=0.83=hab24e00_0 - fontconfig=2.14.1=hc2a2eb6_0 - fonts-conda-ecosystem=1=0 - fonts-conda-forge=1=0 - freetype=2.12.1=hca18f0e_1 - gettext=0.21.1=h27087fc_0 - glib=2.74.1=h6239696_1 - glib-tools=2.74.1=h6239696_1 - gst-plugins-base=1.20.3=h57caac4_2 - gstreamer=1.20.3=hd4edc92_2 - h5py=3.1.0=nompi_py36hc1bc4f5_100 - hdf5=1.10.6=nompi_h6a2412b_1114 - icu=69.1=h9c3ff4c_0 - identify=2.3.7=pyhd8ed1ab_0 - importlib-metadata=4.8.1=py36h5fab9bb_0 - importlib_metadata=4.8.1=hd8ed1ab_1 - importlib_resources=5.4.0=pyhd8ed1ab_0 - intel-openmp=2022.1.0=h9e868ea_3769 - itsdangerous=2.0.1=pyhd8ed1ab_0 - jinja2=3.0.3=pyhd8ed1ab_0 - jpeg=9e=h166bdaf_2 - keyutils=1.6.1=h166bdaf_0 - kiwisolver=1.3.1=py36h605e78d_1 - krb5=1.20.1=hf9c8cef_0 - lcms2=2.12=hddcbb42_0 - ld_impl_linux-64=2.39=hcc3a1bd_1 - lerc=3.0=h9c3ff4c_0 - libblas=3.9.0=16_linux64_mkl - libcblas=3.9.0=16_linux64_mkl - libclang=13.0.1=default_hc23dcda_0 - libcurl=7.87.0=h6312ad2_0 - libdeflate=1.10=h7f98852_0 - libedit=3.1.20191231=he28a2e2_2 - libev=4.33=h516909a_1 - libevent=2.1.10=h9b69904_4 - libffi=3.4.2=h7f98852_5 - libgcc-ng=12.2.0=h65d4601_19 - libgfortran-ng=12.2.0=h69a702a_19 - libgfortran5=12.2.0=h337968e_19 - libglib=2.74.1=h606061b_1 - libgomp=12.2.0=h65d4601_19 - libiconv=1.17=h166bdaf_0 - liblapack=3.9.0=16_linux64_mkl - libllvm13=13.0.1=hf817b99_2 - libnghttp2=1.47.0=hdcd2b5c_1 - libnsl=2.0.0=h7f98852_0 - libogg=1.3.4=h7f98852_1 - libopus=1.3.1=h7f98852_1 - libpng=1.6.39=h753d276_0 - libpq=14.5=h2baec63_4 - libsqlite=3.40.0=h753d276_0 - libssh2=1.10.0=haa6b8db_3 - libstdcxx-ng=12.2.0=h46fd767_19 - libtiff=4.3.0=h0fcbabc_4 - libuuid=2.32.1=h7f98852_1000 - libuv=1.44.2=h166bdaf_0 - libvorbis=1.3.7=h9c3ff4c_0 - libwebp-base=1.2.4=h166bdaf_0 - libxcb=1.13=h7f98852_1004 - libxkbcommon=1.0.3=he3ba5ed_0 - libxml2=2.9.12=h885dcf4_1 - libzlib=1.2.13=h166bdaf_4 - markupsafe=2.0.1=py36h8f6f2f9_0 - matplotlib=3.3.4=py36h5fab9bb_0 - matplotlib-base=3.3.4=py36hd391965_0 - mkl=2022.1.0=hc2b9512_224 - mysql-common=8.0.31=haf5c9bc_0 - mysql-libs=8.0.31=h28c427c_0 - ncurses=6.3=h27087fc_1 - ninja=1.11.0=h924138e_0 - nodeenv=1.6.0=pyhd8ed1ab_0 - nspr=4.35=h27087fc_0 - nss=3.82=he02c5a1_0 - numpy=1.19.5=py36hfc0c790_2 - olefile=0.46=pyh9f0ad1d_1 - openjpeg=2.5.0=h7d73246_0 - openssl=1.1.1s=h0b41bf4_1 - pcre2=10.40=hc3806b6_0 - pillow=8.3.2=py36h676a545_0 - pip=21.3.1=pyhd8ed1ab_0 - pre-commit=2.2.0=py36h9f0ad1d_1 - pthread-stubs=0.4=h36c2ea0_1001 - pycparser=2.21=pyhd8ed1ab_0 - pykdtree=1.3.4=py36ha112f06_0 - pyparsing=3.0.9=pyhd8ed1ab_0 - pyqt=5.12.3=py36h5fab9bb_7 - pyqt-impl=5.12.3=py36h7ec31b9_7 - pyqt5-sip=4.19.18=py36hc4f0c31_7 - pyqtchart=5.12=py36h7ec31b9_7 - pyqtwebengine=5.12.1=py36h7ec31b9_7 - python=3.6.15=hb7a2778_0_cpython - python-dateutil=2.8.2=pyhd8ed1ab_0 - python_abi=3.6=2_cp36m - pytorch=1.9.0=py3.6_cuda11.1_cudnn8.0.5_0 - pytorch-mutex=1.0=cpu - pyyaml=5.4.1=py36h8f6f2f9_1 - qt=5.12.9=h1304e3e_6 - readline=8.1.2=h0f457ee_0 - scipy=1.5.3=py36h81d768a_1 - setuptools=58.0.4=py36h5fab9bb_2 - six=1.16.0=pyh6c4a22f_0 - sqlite=3.40.0=h4ff8645_0 - tk=8.6.12=h27826a3_0 - toml=0.10.2=pyhd8ed1ab_0 - tornado=6.1=py36h8f6f2f9_1 - tqdm=4.45.0=pyh9f0ad1d_1 - typing_extensions=4.1.1=pyha770c72_0 - virtualenv=20.4.7=py36h5fab9bb_0 - werkzeug=2.0.2=pyhd8ed1ab_0 - wheel=0.37.1=pyhd8ed1ab_0 - xorg-libxau=1.0.9=h7f98852_0 - xorg-libxdmcp=1.1.3=h7f98852_0 - xz=5.2.6=h166bdaf_0 - yaml=0.2.5=h7f98852_2 - zipp=3.6.0=pyhd8ed1ab_0 - zlib=1.2.13=h166bdaf_4 - zstd=1.5.2=h6239696_4 - pip: - cffi==1.15.1 - charset-normalizer==2.0.12 - decorator==4.4.2 - docker-pycreds==0.4.0 - gitdb==4.0.9 - gitpython==3.1.18 - googledrivedownloader==0.4 - idna==3.4 - isodate==0.6.1 - joblib==1.1.1 - lmdb==1.0.0 - networkx==2.5.1 - pandas==1.1.5 - pathtools==0.1.2 - promise==2.3 - protobuf==3.19.6 - psutil==5.9.4 - pytz==2022.7 - rdflib==5.0.0 - requests==2.27.1 - scikit-learn==0.24.2 - sentry-sdk==1.12.1 - setproctitle==1.2.3 - shortuuid==1.0.11 - skorch==0.10.0 - smmap==5.0.0 - tabulate==0.8.10 - threadpoolctl==3.1.0 - torch-cluster==1.5.9 - torch-geometric==2.0.3 - torch-scatter==2.0.9 - torch-sparse==0.6.12 - torch-spline-conv==1.2.1 - urllib3==1.26.13 - wandb==0.13.7 - yacs==0.1.8 ```

I wonder if perhaps more tightly-pinned environment files could be provided (or mamba suggested) so that this problem can be circumvented?

[ml-evs]

Ah, just spotted that the conda env is pinning to Python 3.6, which reached end-of-life in 2021. This constraint will definitely need to be relaxed so that others can use the package (and so that dependency versions play nice together)

[ajmedford]

@ml-evs Thanks for your efforts on this, and apologies for the delay. We have run into some serious dependency version conflicts and would like your advice on how to proceed.

In short, it seems that some of the core dependencies (pytorch-geometric and pytoch-sparse) have a version conflict with Python >3.6. @nicoleyghu can elaborate more, but it seems the most practical solution is to pin the versions for everything at Python 3.6 for now and hope that we can upgrade once the dependencies become compatible with newer versions. Let us know what you think.

[ml-evs]

Thanks for checking this out.

From the point of view of the JOSS, I don't think there is any hard editorial requirement that submissions do not use deprecated packages, provided the package is installable at the time of review. If you can provide a working conda environment with fully pinned dependencies on 3.6 then I can give it a go.

From my point of view, I really think it would be worth you putting in the effort to fix the dependency issues (though I really do feel your pain, believe me). In terms of advice for how to proceed with that, you should probably test out (with fresh environments each time) the version ranges you can get away with for the major dependencies, e.g., remove all version pins in your env files (except Python), see which versions get installed, and then test them. If you already know about constraints from your own code (e.g., using a particular version of the torch API), you could include these as minimum/maximum version requirements, rather than hard pins, then try to let conda figure things out. If you want a faster dependency solver you can also try mamba (as linked in my top comment), though this won't get you around the bottleneck of downloading multiple torch versions...

[ajmedford]

Thanks, I will have @nicoleyghu prioritize getting a stable environment with Python 3.6. Then she can work through some of the suggestions for improving the dependency issues as you work through the review. Fully fixing them may be beyond our skills/bandwidth if it involves modifying the pytorch-geometric or -sparse source codes, but it seems like there are some good work-arounds for at least minimizing the impact and improving the robustness of the install process.

@nicoleyghu - please let @ml-evs know when to give this another go.

[nicoleyghu]

@ml-evs Thanks for your comment and suggestions! To update on the environment, the latest commit addressed deprecated Python 3.6 by upgrading to Python 3.9. Please pull again and give the updated env_cpu.yml a go. I tested on OpenCI's docker image and several different machines. The average build time should be around ~13mins with conda. Please let us know if you run into any issues.

Before processing, please make sure that the system has gcc compiler installed (tested on GCC 7.5). Steps:

1. git clone https://github.com/ulissigroup/amptorch.git
2. conda config --set channel_priority strict
3. conda update conda
4. conda env create -f env_cpu.yml
5. conda activate amptorch
6. pip install -e .

[ml-evs]

Thanks @nicoleyghu, I can confirm the package now installs fine with those instructions. I'm now just testing the GPU env too. I've been able to run most of the examples with a bit of tweaking, (e.g., editing the path to a psp file in the GMP example, and making a missing data2.lmdb in the LMDB examples).

I'll now start my review proper and will play around with the package over the next couple of weeks (and try to craft my own examples).

I will default to providing feedback (e.g., on the paper, docs and so on) through issues on this repo --- please let me know if you find this disruptive of the package's development and I can instead switch to writing comments on the JOSS review issue.