atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

problem in pyat build/setup.py #123

Closed swhite2401 closed 5 years ago

swhite2401 commented 5 years ago

Hello all,

We have been facing issues building pyat recently. We believe the problem is related to setup.py where the source code is copied over during the built. For some reason that we don't understand this does not go well and the sources is then not found. The build fails and on top of that it leave the egg-info folder in a bad state that prevents to run the install again. We have temporarily fixed the problem by pulling out the part of the code that copies the source to the pyat folder outside the build in setup.py, see attached file below.

File that are created during built and need to be added to gitignore: pyat/accelerator-toolbox.egg-link pyat/easy-install.pth pyat/f2py pyat/f2py3 pyat/f2py3.7 pyat/site.py

Thank you for your help and best regards, Simon

import os
import glob
import sys
import shutil
try:
    import numpy
except ImportError:
    print('\npyAT requires numpy. '
          'Please install numpy: "pip install numpy"\n')
    sys.exit()
from io import open
from setuptools import setup, Extension, find_packages
from setuptools.command.build_ext import build_ext

here = os.path.abspath(os.path.dirname(__file__))
macros = [('PYAT', None)]

# Get the long description from the README file
with open(os.path.join(here, 'README.rst'), encoding='utf-8') as f:
    long_description = f.read()

class CopyDuringBuild(build_ext):
    """Only copy integrators and diffmatrix during build. This avoids filepath
    issues during wheel building where a temporary copy of pyAT is made.
    """
    def run(self):
        #here = os.path.abspath(os.path.dirname(__file__))
        #integrator_src_orig = os.path.abspath(os.path.join(here, '../atintegrators'))
        #integrator_src = os.path.abspath(os.path.join(here, 'integrator-src'))
        #diffmatrix_source = os.path.abspath(os.path.join(here, '../atmat/atphysics/Radiation'))
        # Copy files into pyat for distribution.
        #source_files = glob.glob(os.path.join(integrator_src_orig, '*.[ch]'))
        #source_files.extend(glob.glob(os.path.join(diffmatrix_source, 'findmpoleraddiffmatrix.c')))
        #if not os.path.exists(integrator_src):
        #    os.makedirs(integrator_src)
        #for f in source_files:
        #    shutil.copy2(f, integrator_src)
        build_ext.run(self)

here = os.path.abspath(os.path.dirname(__file__))
integrator_src_orig = os.path.abspath(os.path.join(here, '../atintegrators'))
integrator_src = os.path.abspath(os.path.join(here, 'integrator-src'))
diffmatrix_source = os.path.abspath(os.path.join(here, '../atmat/atphysics/Radiation'))
# Copy files into pyat for distribution.
source_files = glob.glob(os.path.join(integrator_src_orig, '*.[ch]'))
source_files.extend(glob.glob(os.path.join(diffmatrix_source, 'findmpoleraddiffmatrix.c')))
if not os.path.exists(integrator_src):
   os.makedirs(integrator_src)
for f in source_files:
   shutil.copy2(f, integrator_src)

at_source = os.path.abspath(os.path.join(here,'at.c'))
#integrator_src_orig = os.path.abspath(os.path.join(here, '../atintegrators'))
#integrator_src = os.path.abspath(os.path.join(here, 'integrator-src'))
#diffmatrix_source = os.path.abspath(os.path.join(here, '../atmat/atphysics/Radiation'))
pass_methods = glob.glob(os.path.join(integrator_src, '*Pass.c'))
diffmatrix_method = os.path.join(integrator_src, 'findmpoleraddiffmatrix.c')

cflags = []

if not sys.platform.startswith('win32'):
    cflags += ['-Wno-unused-function']

def integrator_extension(pass_method):
    name, _ = os.path.splitext(os.path.basename(pass_method))
    name = ".".join(('at', 'integrators', name))
    return Extension(name=name,
                     sources=[pass_method],
                     include_dirs=[numpy.get_include(), integrator_src, diffmatrix_source],
                     define_macros=macros,
                     extra_compile_args=cflags)

at = Extension('at.tracking.atpass',
               sources=[at_source],
               define_macros=macros,
               include_dirs=[numpy.get_include(), integrator_src, diffmatrix_source],
               extra_compile_args=cflags)

diffmatrix = Extension(name='at.physics.diffmatrix',
                       sources=[diffmatrix_method],
                       include_dirs=[numpy.get_include(), integrator_src, diffmatrix_source],
                       define_macros=macros,
                       extra_compile_args=cflags)

setup(cmdclass={'build_ext': CopyDuringBuild},
      name='accelerator-toolbox',
      version='0.0.1',
      description='Accelerator Toolbox',
      long_description=long_description,
      author='The AT collaboration',
      author_email='atcollab-general@lists.sourceforge.net',
      url='https://pypi.org/project/accelerator-toolbox/',
      install_requires=['numpy>=1.10', 'scipy>=0.16'],
      packages=find_packages(),
      ext_modules=[at, diffmatrix] + [integrator_extension(pm) for pm in pass_methods],
      zip_safe=False,
      python_requires='>=2.7.4')
willrogers commented 5 years ago

Hi,

Which version of Python and which operating system are you using?

Cheers, Will

lfarv commented 5 years ago

Hi Simon, It looks like there is an overlap between your Git working directory and the location where setup.py install puts the result of the build. Otherwise, the build should not create in the Git working directory the files that you mention to be ignored (accelerator-toolbox.egg-link, easy-install.pth, etc.). The target of installation depends on your OS and configuration. It is something ending in .../site-packages/, either in a virtual python environment, or defined by a.pydistutils.cfg file in the home directory, or else. Your Git working directory should be away from that to avoid conflicts. Can you check that?

swhite2401 commented 5 years ago

Hello, We have tried with python 3.7 and 3.5 and 2.7 all showing the same behavior. There is indeed an overlap, ESRF is pushing to use CONDA which is not compatible with venv as suggested in the README so a local install using the CONDA environment was the simplest for us. We have indeed put the install-dir into the GIT working directory, which is a mistake. I will try to generate a new install-dir and keep you updated. Thanks, Simon

swhite2401 commented 5 years ago

I have done the check suggested by Laurent. The untracked files are indeed put in a different directory and are not an issue anymore. However setup.py needs to run twice because on the first try the source is not found. The suggested modifed setup.py fixes the problem Simon

willrogers commented 5 years ago

This is not quite the same question, but I have just made a test release of accelerator-toolbox 0.0.2 on Test PyPI: https://test.pypi.org/project/accelerator-toolbox/#files

Presumably it would be easier for you to fetch those files than build AT yourself. It would also help to have a bit of testing before I push these files to the real PyPI.

lfarv commented 5 years ago

Hello again, Simon There is still something strange in your configuration. Can you check this:

Also can you run successively, from git_root/pyat:

python setup.py clean python setup.py build python setup.py install

and tell me at which step the problem occurs and how it shows up ? Can you give me more details on the python version you use and on your directory locations? I successfully run the setup script on my account on rnice using the standard system python (2.7) without problems.

willrogers commented 5 years ago

@lfarv I think you must have deleted the comment but anyway: to use test PyPI:

pip install -i https://test.pypi.org/simple/ accelerator-toolbox
swhite2401 commented 5 years ago

Hello,

@lfarv We are running these tests on raki, but seen the same behavior on rnice with my account and the one of @lcarver . The points you noted are all fullfilled, the problem occurs when running:

python setup.py build

In that case all the files use in the loop (setup.py):

for f in source_files: shutil.copy2(f, integrator_src)

are not compiled and shared library are not found. Running install afterwards solves the problem as it seems to run the build command again (??), the sequence:

python setup.py clean python setup.py build python setup.py install

will therefore install the software properly. However the command:

/usr/bin/python3.5 setup.py develop --install-dir=/machfs/swhite/matlab/pyat_build

fails because not all integrators are compiled on the first try.

@willrogers That is interesting, we will test the pip install. However we may modify passmethods and properly building the code ourselves is necessary.

Simon

willrogers commented 5 years ago

You're right, I see this problem as well. I note that in the Travis builds we are running python setup.py build before python setup.py develop which is why we don't see it there.

The build process leaves a lot of stuff behind, so most likely you don't see this after you've run it successfully once.

I'll have a look at your suggestion tomorrow.

lfarv commented 5 years ago

@swhite2401:

I have no problem building on raki. Here is my sequence of commands:

raki2:~ $ cd /machfs/laurent/dev/libraries/at/pyat
...
raki2:pyat $ /usr/bin/python3.5 setup.py build
...
raki2:pyat $ /usr/bin/python3.5 setup.py install --prefix=~/.local
...
raki2:pyat $ cd
raki2:~ $ /usr/bin/python3.5
Python 3.5.3 (default, Sep 27 2018, 17:25:39)
[GCC 6.3.0 20170516] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import at
>>> at.__file__
'/users/laurent/.local/lib/python3.5/site-packages/accelerator_toolbox-0.0.2-py3.5-linux-x86_64.egg/at/__init__.py'
>>>

Here is an extract of the full output: log.txt

I think there are 2 different problems in your case:

For the build configuration:

For the compilation:

It also happened to me that after a number of manipulations in the working space (switching branches, changing python version, etc.), the build sequence does not compile because it finds some already existing files. setup.py clean does not solve this. The brute force solution was to completely delete the pyat/build directory. It then works for some time.

swhite2401 commented 5 years ago

Hello Laurent,

The sequence you use works for me as well. develop will try to install some files in: /usr/local/lib/python3.5/dist-packages/ which we did not want so we combined it with --install-dir to select the destination. .pydistutils.cfg was suppressed.

My feeling is that copying files during build is not working, and it seems @willrogers saw the same behavior.

lfarv commented 5 years ago

The copy also worked for me, but thanks, Simon, I think I understand now !

ˋdevelop` is intended for testing rapidly before installing. It should not put anything outside the working space (I never noticed it did). On the other hand, install needs adequate permissions on the target directory, so the copy is perfectly legal then (this is true not only for pyat, but for any python module).

So I think the copy of the sources should occur only for the ˋinstall` command.

In your case, Simon, unless you want to iterate on testing new things, you need to use build + install, so that the software can be used, and specify the install directory with —user, —prefix= or with a carefully prepared .pydistutils.cfg (which is used for all installations without needing to specify each time).

Will, is it possible to restrict the copy to the install command?

In addition, the compilation may still fail from time to time, but it’s another problem!

swhite2401 commented 5 years ago

Thanks Laurent and Will! I think things are getting clear now. Just one final comment, on the readme instructions for installation suggest to use develop. In case the standard procedure is build+install instructions should probably be modified.

For information, we are starting to use pyAT with/in tango device servers so your work is much appreciated!

lfarv commented 5 years ago

Back home, I checked that develop does indeed put things in the install directory (2 lines linking to the working space). So the problem is not the copy of the sources, and develop must be used with the same --prefix= option than install. No need to change anything in setup.py. I agree with Simon, the standard procedure for normal users should be set in the instructions to build + install, rather that develop.

@swhite2401: glad to see that you are starting working with pyat !

willrogers commented 5 years ago

@swhite2401 it's just possible that you would be interested in https://github.com/dls-controls/pytac, which provides some of the Matlab Middle Layer functionality in Python. We have recently added pyat as a simulator for Pytac.

It is based on EPICS but could be adapted for Tango.

willrogers commented 5 years ago

It is still not entirely clear to me that setup.py is correct because I wouldn't expect to have to run both python setup.py build and python setup.py install. However, it sounds like everyone can continue working at the moment.

T-Nicholls commented 5 years ago

Unfortunately, I'm pretty sure there is a bug in there somewhere as setup.py install should call setup.py build first, like setup.py develop does.

If I remember correctly the reason we run build before develop on Travis that is because of issues with the cibuildwheel environment and were we not trying to build wheels develop would work fine on its own.

On the topic of updating the install instructions, as I understood it the direction we were going was setup.py develop would be the standard installation process for contributors and everyone else would use pip install. This was the reason I ignored the fact that setup.py install no longer worked earlier this year, after I discovered it by accidentally running install instead of develop.

willrogers commented 5 years ago

I have now uploaded accelerator-toolbox 0.0.2 to PyPI.

lfarv commented 5 years ago

Very strange: I have always used setup.py install without problems, and it does trigger a build first if it was not done before. The only problem I had is that in rare occasions (update of numpy, I think, but may be other occasions), it does not detect that the compiled extensions are not outdated and it leaves the old versions without recompiling, which gives runtime errors. That's why I suggested deleting the build directory, which is recreated correctly by a simple setup.py install (no previous build necessary). I never had problems with modified .py files

UPDATE: I tested again on:

In all cases, setup.py install trigged a build ! So no problem for me…

willrogers commented 5 years ago

See #125.

willrogers commented 5 years ago

I believe this problem is now resolved.