Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pyproject.toml #338

Merged
merged 31 commits into from
Oct 10, 2023
Merged

Pyproject.toml #338

merged 31 commits into from
Oct 10, 2023

Conversation

jcapriot
Copy link
Member

This switched the installation method for discretize away from a setup.py style to a pyproject.toml. The purpose of this file is to build a discretize wheel in an isolated python environment, then install that into the local environment. This allows us to cleanly have separate build dependencies, and run-time dependencies.

For discretize the build dependencies are:

  • numpy
  • cython
  • setuptools_scm

the run time requirements are simply:

  • numpy
  • scipy

This has long been a recurring issue that we were mostly solving by telling people to install with conda-forge.

The new build process makes use of a meson build backend (instead of setuptools) to build and install all of the extensions. This is a pretty big change for discretize, however... this is already successfully done with scipy, so I don't think we should have any issues in the long run. I intend on adding more details to the installation steps that we need to take.

@codecov
Copy link

codecov bot commented Sep 11, 2023

Codecov Report

Merging #338 (3357e9e) into main (615d1b3) will decrease coverage by 0.36%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main     #338      +/-   ##
==========================================
- Coverage   85.47%   85.12%   -0.36%     
==========================================
  Files          39       36       -3     
  Lines       12001     8408    -3593     
==========================================
- Hits        10258     7157    -3101     
+ Misses       1743     1251     -492     
Files Coverage Δ
discretize/operators/inner_products.py 90.63% <ø> (ø)
discretize/__init__.py 62.50% <50.00%> (+1.63%) ⬆️

... and 3 files with indirect coverage changes

@omid-b
Copy link
Contributor

omid-b commented Sep 15, 2023

Thank you very much for this work. This is certainly a great step for discretize toward the right direction, and will make discretize more future-proof. I was very happy to see this PR after our discussion about this before, so I immediately did fetch and tried to install on my local Linux machine.

Despite that it looks like everything should work as intended (successful installation message on a fresh pip env), It did not work for me. Please see below:

Method 1 of 2: non-editable installation

$ virtualenv env --python=3.11
created virtual environment CPython3.11.0.candidate.1-64 in 3065ms
  creator CPython3Posix(dest=/mnt/c/Users/baghom/Desktop/discretize/env, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/omid-b/.local/share/virtualenv)
    added seed packages: pip==22.0.2, setuptools==59.6.0, wheel==0.37.1
  activators BashActivator,CShellActivator,FishActivator,NushellActivator,PowerShellActivator,PythonActivator

$ source env/bin/activate

$ pip install .
Processing /mnt/c/Users/baghom/Desktop/discretize
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Installing backend dependencies ... done
  Preparing metadata (pyproject.toml) ... done
Collecting scipy>=1.8
  Using cached scipy-1.11.2-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (36.3 MB)
Collecting numpy>=1.22.4
  Using cached numpy-1.25.2-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (18.2 MB)
Building wheels for collected packages: discretize
  Building wheel for discretize (pyproject.toml) ... done
  Created wheel for discretize: filename=discretize-0.9.1.dev69+gc35d6061-cp311-cp311-linux_x86_64.whl size=4479981 sha256=b3b70713a0525350f06dd6f5bfde47fb922228765c402f5c38c00c3ddafe7fa0
  Stored in directory: /tmp/pip-ephem-wheel-cache-8kcc9xyn/wheels/24/51/d7/3c5cfdc294c10a59a01f294f49d8c12c2378731ab132315af9
Successfully built discretize
Installing collected packages: numpy, scipy, discretize
Successfully installed discretize-0.9.1.dev69+gc35d6061 numpy-1.25.2 scipy-1.11.2

$ python -c "import discretize"
cannot import name 'interputils_cython' from 'discretize._extensions' (/mnt/c/Users/baghom/Desktop/discretize/discretize/_extensions/__init__.py)
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/mnt/c/Users/baghom/Desktop/discretize/discretize/__init__.py", line 42, in <module>
    from discretize.unstructured_mesh import SimplexMesh
  File "/mnt/c/Users/baghom/Desktop/discretize/discretize/unstructured_mesh.py", line 7, in <module>
    from discretize._extensions.simplex_helpers import (
ModuleNotFoundError: No module named 'discretize._extensions.simplex_helpers'

Method 2 of 2: editable installation

$ virtualenv venv --python=3.11
created virtual environment CPython3.11.0.candidate.1-64 in 2367ms
  creator CPython3Posix(dest=/mnt/c/Users/baghom/Desktop/discretize/venv, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, setuptools=bundle, wheel=bundle, via=copy, app_data_dir=/home/omid-b/.local/share/virtualenv)
    added seed packages: pip==22.0.2, setuptools==59.6.0, wheel==0.37.1
  activators BashActivator,CShellActivator,FishActivator,NushellActivator,PowerShellActivator,PythonActivator

$ source venv/bin/activate

$ pip install -e .
Obtaining file:///mnt/c/Users/baghom/Desktop/discretize
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Installing backend dependencies ... done
  Preparing editable metadata (pyproject.toml) ... done
Collecting numpy>=1.22.4
  Using cached numpy-1.25.2-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (18.2 MB)
Collecting scipy>=1.8
  Using cached scipy-1.11.2-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (36.3 MB)
Building wheels for collected packages: discretize
  Building editable for discretize (pyproject.toml) ... done
  Created wheel for discretize: filename=discretize-0.9.1.dev69+gc35d6061-cp311-cp311-linux_x86_64.whl size=20809 sha256=0b4ff2cc5605ce5364d26f4fbeec304575934cc3c4717860fea62c27f420e60b
  Stored in directory: /tmp/pip-ephem-wheel-cache-_ovyp0rh/wheels/24/51/d7/3c5cfdc294c10a59a01f294f49d8c12c2378731ab132315af9
Successfully built discretize
Installing collected packages: numpy, scipy, discretize
Successfully installed discretize-0.9.1.dev69+gc35d6061 numpy-1.25.2 scipy-1.11.2

$ python -c "import discretize"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 1178, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1140, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 1080, in _find_spec
  File "/mnt/c/Users/baghom/Desktop/discretize/venv/lib/python3.11/site-packages/_discretize_editable_loader.py", line 271, in find_spec
    tree = self.rebuild()
           ^^^^^^^^^^^^^^
  File "/mnt/c/Users/baghom/Desktop/discretize/venv/lib/python3.11/site-packages/_discretize_editable_loader.py", line 312, in rebuild
    subprocess.run(self._build_cmd, cwd=self._build_path, env=env, stdout=stdout, check=True)
  File "/usr/lib/python3.11/subprocess.py", line 546, in run
    with Popen(*popenargs, **kwargs) as process:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/subprocess.py", line 1022, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.11/subprocess.py", line 1899, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/pip-build-env-yjgyh3lm/normal/bin/ninja'

@jcapriot
Copy link
Member Author

It looks like from your first attempt at installing it, it installed successfully, but you likely tried to import it while you were still in the source code folder, and thus python's order of searching for packages meant it tried to import the one that was local to your current working directory. I suspect you can import discretize from anywhere else though.

The steps for an editable install are not the same as before (which I will update the details of what is needed for developers to perform this step), and a simple pip install -e . will likely not work.

@jcapriot
Copy link
Member Author

For everyone looking into this, the method of performing an editable install (intended only for developers) is:

  1. Install the build backend requirements:
    pip install meson-python meson ninja

  2. install the discretize build and runtime requirements:
    pip install spicy numpy setuptools-scm cython

  3. install an editable discretize without build isolation:
    pip install --no-build-isolation --editable .

These steps are detailed in the meson-python documentation on editable installs https://meson-python.readthedocs.io/en/latest/how-to-guides/editable-installs.html

@omid-b
Copy link
Contributor

omid-b commented Sep 18, 2023

@jcapriot thanks for your explanation. Just to report back to you:

  • The first method worked after I changed directory (if discretize was a src-layout, we did not have this issue though).
  • The second method also worked after I followed your instructions

@jcapriot jcapriot requested review from lheagy and santisoler October 2, 2023 18:25
@jcapriot
Copy link
Member Author

jcapriot commented Oct 2, 2023

@santisoler I think this is about ready. I'd appreciate if you could take a look through it, especially regarding the new installation instruction page. It could likely be expanded in a similar manner to what we have on SimPEG, but for this PR, my concern is to at least have it be correct and then expand on it in a future version.

Copy link
Member

@santisoler santisoler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking nice @jcapriot! Having mason doing the venv installation of build dependencies and handling the compilation is neat!

I've left a few suggestions, mainly on the documentation on how to install it from the source code and in editable mode. I would also change the use of single backticks to double backticks when mentioning some packages and commands. I've made a few of these changes in the suggestions, but feel free to carry on with the rest.

I followed the instructions for installing from source code and noticed two things:

  1. When installing from source code (not editable mode), you cannot import discretize from within the directory of the repo, since it will try to import modules from the local discretize directory instead from the one installed by pip. Maybe we could add an admonition warning users of this.
  2. The editable installation works perfectly. But I made some changes to the tree_ext.pyx and those changes weren't pulled the next time I imported discretize. Apparently the recompilation is not being done. I leave a small working example so you can test it yourself.

Let's say I have the following script:

import numpy as np
from discretize import TreeMesh
from discretize.utils import mkvc

dh = 5  # minimum cell width (base mesh cell width)
nbc = 64  # number of base mesh cells in x

# Define base mesh (domain and finest discretization)
h = dh * np.ones(nbc)
mesh = TreeMesh([h, h])

# Define corner points for rectangular box
xp, yp = np.meshgrid([120.0, 240.0], [80.0, 160.0])
xy = np.c_[mkvc(xp), mkvc(yp)]  # mkvc creates vectors

# Discretize to finest cell size within rectangular box, with padding in the z direction
# at the finest and second finest levels.
padding = [[0, 2], [0, 2]]
mesh.refine_bounding_box(xy, level=-1, padding_cells_by_level=padding)

cell = mesh[0]
print(cell.nodes)

If I run it with the latest version of discretize, I get:

[0, 1, 2, 4]

Now, let's change some line in the nodes method from the TreeCell:

sed -i '89s/0/1/' discretize/_extensions/tree_ext.pyx

If I rerun the script, I still get the same result, instead of [1, 1, 2, 3]. Apparently the rebuild is not being triggered. I'm not sure how to solve this.

UPDATE on 2023-10-10

After trying this workflow again I found it works just fine in two different computers. So feel free to ignore this non-existent issue. I'm leaving the comment here just in case.

docs/content/installing.rst Outdated Show resolved Hide resolved
docs/content/installing.rst Outdated Show resolved Hide resolved
docs/content/installing.rst Outdated Show resolved Hide resolved
docs/content/installing.rst Outdated Show resolved Hide resolved
docs/content/installing.rst Outdated Show resolved Hide resolved
docs/content/installing.rst Outdated Show resolved Hide resolved
docs/content/installing.rst Outdated Show resolved Hide resolved
@jcapriot
Copy link
Member Author

jcapriot commented Oct 9, 2023

The editable install works fine on my side with your test script. Try using the following command to verify it is actually performing a step to check the installation with importing the editable discretize:

pip install --no-build-isolation --config-settings=editable-verbose=true --editable .

It should give you some feedback about checking if any module needs to be rebuilt every time you import discretize. If not there's some other problem locally with your installation. Try cleaning out all of your old files as I can't particularly predict how it would behave if there were already built modules from before the meson-python build.

jcapriot and others added 3 commits October 9, 2023 09:54
@jcapriot jcapriot merged commit ba586b7 into simpeg:main Oct 10, 2023
15 of 17 checks passed
@jcapriot jcapriot deleted the pyproject branch October 10, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants