manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
163 stars 50 forks source link

Corrfunc does on install correctly from Pypi on a Python-3-only system #208

Closed olebole closed 4 years ago

olebole commented 4 years ago

General information

Issue description

When I try to install Corrfunc from Pypi with only Python 3 installed (no Python 2 executable), I get the following error:

Collecting Corrfunc (from -r requirements.txt (line 1))
  Downloading https://files.pythonhosted.org/packages/33/60/487aba84ade8a0bbf21b0c08fa933ed9c1fa119b89f4d0bdeeb4869fbfaf/Corrfunc-2.3.2.tar.gz (26.1MB)
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-xtgoum6c/Corrfunc/setup.py", line 573, in <module>
        setup_packages()
      File "/tmp/pip-install-xtgoum6c/Corrfunc/setup.py", line 457, in setup_packages
        common_dict = requirements_check()
      File "/tmp/pip-install-xtgoum6c/Corrfunc/setup.py", line 224, in requirements_check
        stdout=subprocess.PIPE, stderr=subprocess.PIPE)
      File "/tmp/pip-install-xtgoum6c/Corrfunc/setup.py", line 70, in run_command
        raise RuntimeError(msg)
    RuntimeError: command = python -c 'from __future__ import print_function; import sys; print(sys.executable)' failed with stdout = b'' stderr = b'/bin/sh: 1: python: not found\n' status 127

I already installed make and libgsl-dev.

Expected behavior

I expect that it just installs ;-)

Actual behavior

see above

olebole commented 4 years ago

When I install Python 2, it still does not compile:

$ pip3 install -r requirements.txt
Collecting Corrfunc (from -r requirements.txt (line 1))
  Downloading https://files.pythonhosted.org/packages/33/60/487aba84ade8a0bbf21b0c08fa933ed9c1fa119b89f4d0bdeeb4869fbfaf/Corrfunc-2.3.2.tar.gz (26.1MB)
Requirement already satisfied: astropy in /usr/lib/python3/dist-packages (from -r requirements.txt (line 2)) (3.1.2)
Collecting healpy (from -r requirements.txt (line 3))
  Downloading https://files.pythonhosted.org/packages/43/85/01fb87b670aa34437eebc71e93212e73bb338565cfa134f6883061152840/healpy-1.13.0-cp37-cp37m-manylinux1_x86_64.whl (11.8MB)
Requirement already satisfied: matplotlib in /usr/lib/python3/dist-packages (from -r requirements.txt (line 4)) (3.0.2)
Requirement already satisfied: numpy in /usr/lib/python3/dist-packages (from -r requirements.txt (line 5)) (1.16.2)
Requirement already satisfied: pandas in /usr/lib/python3/dist-packages (from -r requirements.txt (line 6)) (0.23.3+dfsg)
Requirement already satisfied: scipy in /usr/lib/python3/dist-packages (from -r requirements.txt (line 7)) (1.1.0)
Requirement already satisfied: statsmodels in /usr/lib/python3/dist-packages (from -r requirements.txt (line 8)) (0.8.0)
Requirement already satisfied: future in /usr/lib/python3/dist-packages (from Corrfunc->-r requirements.txt (line 1)) (0.16.0)
Collecting wurlitzer (from Corrfunc->-r requirements.txt (line 1))
  Downloading https://files.pythonhosted.org/packages/24/5e/f3bd8443bfdf96d2f5d10097d301076a9eb55637b7864e52d2d1a4d8c72a/wurlitzer-2.0.0-py2.py3-none-any.whl
Requirement already satisfied: six in /usr/lib/python3/dist-packages (from healpy->-r requirements.txt (line 3)) (1.12.0)
Building wheels for collected packages: Corrfunc
  Running setup.py bdist_wheel for Corrfunc: started
  Running setup.py bdist_wheel for Corrfunc: finished with status 'error'
  Complete output from command /usr/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /tmp/pip-wheel-jb4bqwvm --python-tag cp37:
  running bdist_wheel
  running build
  running build_py
  creating build
  creating build/lib.linux-x86_64-3.7
  creating build/lib.linux-x86_64-3.7/Corrfunc
  copying Corrfunc/call_correlation_functions_mocks.py -> build/lib.linux-x86_64-3.7/Corrfunc
  copying Corrfunc/utils.py -> build/lib.linux-x86_64-3.7/Corrfunc
  copying Corrfunc/call_correlation_functions.py -> build/lib.linux-x86_64-3.7/Corrfunc
  copying Corrfunc/__init__.py -> build/lib.linux-x86_64-3.7/Corrfunc
  copying Corrfunc/tests.py -> build/lib.linux-x86_64-3.7/Corrfunc
  copying Corrfunc/io.py -> build/lib.linux-x86_64-3.7/Corrfunc
  creating build/lib.linux-x86_64-3.7/Corrfunc/mocks
  copying Corrfunc/mocks/DDtheta_mocks.py -> build/lib.linux-x86_64-3.7/Corrfunc/mocks
  copying Corrfunc/mocks/DDsmu_mocks.py -> build/lib.linux-x86_64-3.7/Corrfunc/mocks
  copying Corrfunc/mocks/__init__.py -> build/lib.linux-x86_64-3.7/Corrfunc/mocks
  copying Corrfunc/mocks/vpf_mocks.py -> build/lib.linux-x86_64-3.7/Corrfunc/mocks
  copying Corrfunc/mocks/DDrppi_mocks.py -> build/lib.linux-x86_64-3.7/Corrfunc/mocks
  creating build/lib.linux-x86_64-3.7/Corrfunc/theory
  copying Corrfunc/theory/DDsmu.py -> build/lib.linux-x86_64-3.7/Corrfunc/theory
  copying Corrfunc/theory/DD.py -> build/lib.linux-x86_64-3.7/Corrfunc/theory
  copying Corrfunc/theory/__init__.py -> build/lib.linux-x86_64-3.7/Corrfunc/theory
  copying Corrfunc/theory/xi.py -> build/lib.linux-x86_64-3.7/Corrfunc/theory
  copying Corrfunc/theory/vpf.py -> build/lib.linux-x86_64-3.7/Corrfunc/theory
  copying Corrfunc/theory/wp.py -> build/lib.linux-x86_64-3.7/Corrfunc/theory
  copying Corrfunc/theory/DDrppi.py -> build/lib.linux-x86_64-3.7/Corrfunc/theory
  running egg_info
  writing Corrfunc.egg-info/PKG-INFO
  writing dependency_links to Corrfunc.egg-info/dependency_links.txt
  writing requirements to Corrfunc.egg-info/requires.txt
  writing top-level names to Corrfunc.egg-info/top_level.txt
  reading manifest file 'Corrfunc.egg-info/SOURCES.txt'
  reading manifest template 'MANIFEST.in'
  warning: no files found matching 'INSTALL'
  warning: no files found matching 'docs'
  warning: no previously-included files found matching 'theory/tests/data/random_Zspace.ff'
  warning: no previously-included files found matching 'mocks/tests/data/Mr19_randoms_northonly.rdcz.ff'
  writing manifest file 'Corrfunc.egg-info/SOURCES.txt'
  creating build/lib.linux-x86_64-3.7/theory
  creating build/lib.linux-x86_64-3.7/theory/tests
  creating build/lib.linux-x86_64-3.7/theory/tests/data
  copying Corrfunc/../theory/tests/data/gals_Mr19.ff -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests/data
  copying Corrfunc/../theory/tests/data/cmassmock_Zspace.ff -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests/data
  creating build/lib.linux-x86_64-3.7/mocks
  creating build/lib.linux-x86_64-3.7/mocks/tests
  creating build/lib.linux-x86_64-3.7/mocks/tests/data
  copying Corrfunc/../mocks/tests/data/Mr19_centers_xyz_forVPF_rmax_10Mpc.txt -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests/data
  copying Corrfunc/../mocks/tests/data/Mr19_mock_northonly.rdcz.ff -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests/data
  copying Corrfunc/../mocks/tests/data/Mr19_mock_northonly.rdcz.dat -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests/data
  copying Corrfunc/../theory/tests/Mr19_DD_nonperiodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/cmass_DR_nonperiodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_DDrppi_nonperiodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/cmass_DD_periodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/cmass_DR_periodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_DDsmu_nonperiodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_DDsmu_periodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/bins -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_DDrppi_periodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_vpf_periodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_xi -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_wp -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/Mr19_DD_periodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../theory/tests/cmass_RR_periodic -> build/lib.linux-x86_64-3.7/Corrfunc/../theory/tests
  copying Corrfunc/../mocks/tests/Mr19_mock.DR -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/Mr19_mock_DDsmu.RR -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/Mr19_mock_DDsmu.DR -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/Mr19_mock_wtheta.DR -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/Mr19_mock_vpf -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/bins -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/Mr19_mock_wtheta.DD -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/Mr19_mock.DD -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/angular_bins -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  copying Corrfunc/../mocks/tests/Mr19_randoms_vpf -> build/lib.linux-x86_64-3.7/Corrfunc/../mocks/tests
  running build_ext
  tput: No value for $TERM and no -T specified
  tput: No value for $TERM and no -T specified
  tput: No value for $TERM and no -T specified
  tput: No value for $TERM and no -T specified
  tput: No value for $TERM and no -T specified
  tput: No value for $TERM and no -T specified
  ../../common.mk:480: "PYTHON" is set to /usr/bin/python3; using "/usr/bin/python-config" as python-config. If this is not correct, please also set "PYTHON_CONFIG_EXE" in "common.mk" to appropriate python-config
  ../../common.mk:486: *** python-config ("/usr/bin/python-config") not found. Please set PYTHON_CONFIG_EXE in "common.mk" to appropriate python-config before installing Corrfunc.2.3.2. Installing python-devel might fix this issue .  Stop.
  Traceback (most recent call last):
    File "<string>", line 1, in <module>
    File "/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py", line 573, in <module>
      setup_packages()
    File "/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py", line 565, in setup_packages
      setup(**metadata)
    File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 145, in setup
      return distutils.core.setup(**attrs)
    File "/usr/lib/python3.7/distutils/core.py", line 148, in setup
      dist.run_commands()
    File "/usr/lib/python3.7/distutils/dist.py", line 966, in run_commands
      self.run_command(cmd)
    File "/usr/lib/python3.7/distutils/dist.py", line 985, in run_command
      cmd_obj.run()
    File "/usr/lib/python3/dist-packages/wheel/bdist_wheel.py", line 188, in run
      self.run_command('build')
    File "/usr/lib/python3.7/distutils/cmd.py", line 313, in run_command
      self.distribution.run_command(command)
    File "/usr/lib/python3.7/distutils/dist.py", line 985, in run_command
      cmd_obj.run()
    File "/usr/lib/python3.7/distutils/command/build.py", line 135, in run
      self.run_command(cmd_name)
    File "/usr/lib/python3.7/distutils/cmd.py", line 313, in run_command
      self.distribution.run_command(command)
    File "/usr/lib/python3.7/distutils/dist.py", line 985, in run_command
      cmd_obj.run()
    File "/usr/lib/python3/dist-packages/setuptools/command/build_ext.py", line 78, in run
      _build_ext.run(self)
    File "/usr/lib/python3.7/distutils/command/build_ext.py", line 340, in run
      self.build_extensions()
    File "/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py", line 353, in build_extensions
      run_command(command)
    File "/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py", line 70, in run_command
      raise RuntimeError(msg)
  RuntimeError: command = cd theory/python_bindings && make  failed with stdout = None stderr = None status 2

  ----------------------------------------
  Running setup.py clean for Corrfunc
  Failed building wheel for Corrfunc
Failed to build Corrfunc
Installing collected packages: wurlitzer, Corrfunc, healpy
  Running setup.py install for Corrfunc: started
    Running setup.py install for Corrfunc: finished with status 'error'
    Complete output from command /usr/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-record-csrhzb8o/install-record.txt --single-version-externally-managed --compile:
    mkdir -p lib
    mkdir -p bin
    mkdir -p include
    make -C theory libs
    make[1]: Entering directory '/tmp/pip-install-m8cf6bmp/Corrfunc/theory'
    tput: No value for $TERM and no -T specified
    tput: No value for $TERM and no -T specified
    tput: No value for $TERM and no -T specified
    tput: No value for $TERM and no -T specified
    tput: No value for $TERM and no -T specified
    tput: No value for $TERM and no -T specified
    ../common.mk:480: "PYTHON" is set to /usr/bin/python3; using "/usr/bin/python-config" as python-config. If this is not correct, please also set "PYTHON_CONFIG_EXE" in "common.mk" to appropriate python-config
    ../common.mk:486: *** python-config ("/usr/bin/python-config") not found. Please set PYTHON_CONFIG_EXE in "common.mk" to appropriate python-config before installing Corrfunc.2.3.2. Installing python-devel might fix this issue .  Stop.
    make[1]: Leaving directory '/tmp/pip-install-m8cf6bmp/Corrfunc/theory'
    make: *** [Makefile:19: libs] Error 2
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py", line 573, in <module>
        setup_packages()
      File "/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py", line 476, in setup_packages
        run_command(command)
      File "/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py", line 70, in run_command
        raise RuntimeError(msg)
    RuntimeError: command = make libs  failed with stdout = None stderr = None status 2

    ----------------------------------------
Command "/usr/bin/python3 -u -c "import setuptools, tokenize;__file__='/tmp/pip-install-m8cf6bmp/Corrfunc/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-record-csrhzb8o/install-record.txt --single-version-externally-managed --compile" failed with error code 1 in /tmp/pip-install-m8cf6bmp/Corrfunc/
ERROR: Job failed: exit code 1

It looks that the python-config executable is wrongly discovered (should be python3-config).

olebole commented 4 years ago

Another issue (when I pragmatically also install python-dev package):

  countpairs_kernels_double.c:1022:37: error: ‘r’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
                           rpavg[kbin] += r;
                                       ^~
  countpairs_kernels_double.c: In function ‘countpairs_avx_intrinsics_double’:
  countpairs_kernels_double.c:572:37: error: ‘r’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
                           rpavg[kbin] += r;
                                       ^~
  countpairs_kernels_double.c: In function ‘countpairs_sse_intrinsics_double’:
  countpairs_kernels_double.c:855:35: error: ‘r’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
                         rpavg[kbin] += r;
                                     ^~
  countpairs_impl_double.c: At top level:
  cc1: error: unrecognized command line option ‘-Wno-unknown-warning-option’ [-Werror]
  cc1: all warnings being treated as errors
  make[1]: *** [../../rules.mk:61: countpairs_impl_double.o] Error 1
  make[1]: Leaving directory '/tmp/pip-install-rju9tl5k/Corrfunc/theory/DD'
  make: *** [Makefile:55: ../../theory/DD/libcountpairs.a] Error 2
manodeep commented 4 years ago

Thanks @olebole for reporting the issues. During install, Corrfunc does assume that the default python executable is python (rather than python3 or python2). While that default can be changed by editing common.mk when installing from source, this editing is not possible under pip -- hence I would expect to see the error that you report. This is also because we do not require python at all for the installation, and hence, the Corrfunc install should proceed even in absence of python. That's why we can't automatically detect what the python executable is; but perhaps there is a way to pass that python executable name (i.e., python3/python) through the setup.py.

There is an implicit dependence on python-config and consequently the python-devel package. However, we might be able to get rid of that dependence by manually printing out the --includes, and other calls made to python(3)-config.

For the last case, I assume you are compiling with -Werror, and that's why that warning has been treated as an error. I have seen that warning raised by gcc7, and that is a compiler bug. There is no scenario where the bin update is being done and r is uninitialised.

olebole commented 4 years ago

In Debian (and also in Ubuntu), it was decided that /usr/bin/python is always Python 2. My problem here is that I have Corrfunc as a requirement of another package, and therefore would like to have it installable via pip. Since also Python 2 reached end of life now, maybe it would be possible to drop Python 2 support? Other packages (like astropy, numpy etc) also did this step. For the error message at the end, I use the default flags when running pip on Debian.

lgarrison commented 4 years ago

Concerning the original error of failing to install if only python3 is present and not python, I think it makes sense for the Makefile to try the former if the latter is not found. And we could inherit the PYTHON environment variable so a pip user could use that as an override to the automatic detection.

I'm kind of confused as to what's happening in the second failure. You installed Python 2, and suddenly it's finding /usr/bin/python3? Weird!

And in the third failure, the crash-on-warning, I'm confused as well. Maybe -Werror is activated via a compiler alias or wrapper; it's only activated from our Makefile if we're on CI. So maybe we're accidentally triggering that, or incorrectly setting another compiler flag? We'd need to see the full log to be able to tell; in particular, the compiler invocation.

olebole commented 4 years ago

Here is a complete log of pip3 install -r requirements.txt : corrlog.txt. What I did exactly is

So, I didn't setup a specific CFLAGS or similar. I agree that it looks weird that it finds Python 3, but pip should know the version it runs under. Maybe this could be used here?

lgarrison commented 4 years ago

Thanks, it does look like Corrfunc thinks it's on CI, either because CI=true or TRAVIS=true. Is this a docker on Travis, by any chance?

@manodeep I guess we need to distinguish between our builds on Travis, and other people's!

olebole commented 4 years ago

No, it is our own. But it is for the installation during the CI of another package. "CI" does not necessarily mean that it is Corrfunc that one wants to test ;-)

lgarrison commented 4 years ago

Yep, we definitely need to fix that!

On Mon, Jan 20, 2020 at 11:34 AM Ole Streicher notifications@github.com wrote:

No, it is our own. But it is for the installation during the CI of another package. "CI" does not necessarily mean that it is Corrfunc that one wants to test ;-)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/issues/208?email_source=notifications&email_token=ABLA7S3DFAPNGK7T5RO5YO3Q6XHA3A5CNFSM4KJASWQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJNGMEA#issuecomment-576349712, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLA7SYV6TFPF5HEXVHLDS3Q6XHA3ANCNFSM4KJASWQQ .

-- Lehman Garrison lgarrison@flatironinstitute.org Flatiron Research Fellow, Cosmology X Data Science Group Center for Computational Astrophysics, Flatiron Institute lgarrison.github.io

manodeep commented 4 years ago

@lgarrison The easiest way would be to export ON_CI from .travis.yml; that would only take effect when we are testing Corrfunc.

@lgarrison I can roll out fixes for most of the issues, but one thing I noticed was the warning from tput about an $TERM not being set. Do you know how to fix/suppress that warning?

lgarrison commented 4 years ago

I haven't seen that tput warning before, but I think we could check whether the terminal supports colors with tput colors. If the exit code is non-zero, then we can no-op the color macros.

manodeep commented 4 years ago

Wouldn't tput colors also require a terminal type? If that's the case, then perhaps we can forego colours entirely if TERM is not set

lgarrison commented 4 years ago

Yes, tput colors will exit with a non-zero exit code if $TERM is not set, thus giving us the information we want. We could check for $TERM instead, but that seems less robust, in the sense that perhaps there are other reasons tput could fail or succeed other than just $TERM.

On Mon, Jan 20, 2020 at 3:09 PM Manodeep Sinha notifications@github.com wrote:

Wouldn't tput colors also require a terminal type? If that's the case, then perhaps we can forego colours entirely if TERM is not set

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/issues/208?email_source=notifications&email_token=ABLA7S64IQSH2JF6RFTUG4TQ6YAHLA5CNFSM4KJASWQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJNWKZY#issuecomment-576415079, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLA7S6FFTCR7FO446VWUJTQ6YAHLANCNFSM4KJASWQQ .

-- Lehman Garrison lgarrison@flatironinstitute.org Flatiron Research Fellow, Cosmology X Data Science Group Center for Computational Astrophysics, Flatiron Institute lgarrison.github.io

manodeep commented 4 years ago

Currently all I am doing is the following:

  CORRFUNC_TERM := $(or ${TERM},${TERM},xterm)  # Take the env. var TERM, or default to xterm                                                                          
  ccred:=$(shell tput -T${CORRFUNC_TERM} setaf 1)
  ccmagenta:=$(shell tput -T${CORRFUNC_TERM} setaf 5)
  ccgreen:=$(shell tput -T${CORRFUNC_TERM} setaf 2)
  ccblue:=$(shell tput -T${CORRFUNC_TERM} setaf 4)
  ccreset:=$(shell tput -T${CORRFUNC_TERM} sgr0)
  boldfont:=$(shell tput -T${CORRFUNC_TERM} bold)
lgarrison commented 4 years ago

Is using -Txterm always safe? I have no idea. I would do something like:

=============== HAVE_TPUT_COLORS := $(shell tput colors &>/dev/null && echo "yes" || echo "no") ifeq (HAVE_TPUT_COLORS,yes) [usual tput definitions] else

No color support; make all color commands no-ops

ccred:=
ccmagenta:=
[etc ... ]

endif

Completely untested, but that's the general idea!

On Mon, Jan 20, 2020 at 5:31 PM Manodeep Sinha notifications@github.com wrote:

Currently all I am doing is the following:

CORRFUNC_TERM := $(or ${TERM},${TERM},xterm) # Take the env. var TERM, or default to xterm ccred:=$(shell tput -T${CORRFUNC_TERM} setaf 1) ccmagenta:=$(shell tput -T${CORRFUNC_TERM} setaf 5) ccgreen:=$(shell tput -T${CORRFUNC_TERM} setaf 2) ccblue:=$(shell tput -T${CORRFUNC_TERM} setaf 4) ccreset:=$(shell tput -T${CORRFUNC_TERM} sgr0) boldfont:=$(shell tput -T${CORRFUNC_TERM} bold)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/issues/208?email_source=notifications&email_token=ABLA7S7HSNIDKNBOG7OPCVTQ6YQ45A5CNFSM4KJASWQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJN6XRA#issuecomment-576449476, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLA7SYSBJ4GALDO4A5HGHLQ6YQ45ANCNFSM4KJASWQQ .

-- Lehman Garrison lgarrison@flatironinstitute.org Flatiron Research Fellow, Cosmology X Data Science Group Center for Computational Astrophysics, Flatiron Institute lgarrison.github.io

lgarrison commented 4 years ago

@olebole, we've pushed some changes to master that we hope will fix this issue. Could you try to install Corrfunc in the environment that was failing before by cloning the repo and running python setup.py install? It's not exactly a pip install, but before we make a pip release, it seems worth it to check that the issue looks resolved this way.

Also, if you or @manodeep know a better way than setup.py install to emulate a pip install, then we should do that instead!

manodeep commented 4 years ago

@lgarrison We could install with pip install . instead of python setup.py install (--user)

lgarrison commented 4 years ago

Good idea, forgot you could do that!

On Thu, Jan 23, 2020 at 6:56 PM Manodeep Sinha notifications@github.com wrote:

@lgarrison https://github.com/lgarrison We could install with pip install . instead of python setup.py install (--user)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/issues/208?email_source=notifications&email_token=ABLA7S664QERAVTKOJAVCPDQ7IVDHA5CNFSM4KJASWQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJZI75Y#issuecomment-577933303, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLA7SZMZRKVSN3CWYVJDJDQ7IVDHANCNFSM4KJASWQQ .

-- Lehman Garrison lgarrison@flatironinstitute.org Flatiron Research Fellow, Cosmology X Data Science Group Center for Computational Astrophysics, Flatiron Institute lgarrison.github.io

olebole commented 4 years ago

@lgarrison I can confirm that I can do a setup.py install on the master branch. Everything compiles. Sorry, I didn't test this properly (I did it on a machine where /usr/bin/python was available and pointed to Python 2). @manodeep for some reason, pip3 insists in downloading a package that is readily compiled. I didn't find an option to disable this.

lgarrison commented 4 years ago

Great! @manodeep I think this means we should make a release, then @olebole can check that it works straight from pip, then we can close this.

manodeep commented 4 years ago

@olebole Thanks for reporting back that the bug-fix did work. I will push out a release in the next day or so.

That compilation is a feature - Corrfunc is always compiled on the install platform from the source distribution (i.e., we do not distribute wheels). This ensures that Corrfunc runs with the highest CPU instructions available on the installed architecture.

@olebole If the compilation is really trouble-some, then there is an option to install through conda - either from the conda-forge or the bccp conda channels -- conda install -c conda-forge corrfunc should do the trick. Please note that those packages will install corrfunc (lower-case C) and will require slight modification of the scripts. There might also be some runtime performance regressions with such an install method, please see this issue

manodeep commented 4 years ago

@olebole Would you mind confirming if pip install . also works for you? We might tweak the install instructions and switch to pip install . (rather than python setup.py ....)

olebole commented 4 years ago

@manodeep It still does not work. I must also withdraw my previous statement, since I accidently did it on a machine that had /usr/bin/python installed.

$ pip3 install -r requirements.txt
Collecting git+https://github.com/manodeep/Corrfunc (from -r requirements.txt (line 5))
  Cloning https://github.com/manodeep/Corrfunc to /tmp/pip-req-build-jnuak6qv
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-req-build-jnuak6qv/setup.py", line 573, in <module>
        setup_packages()
      File "/tmp/pip-req-build-jnuak6qv/setup.py", line 457, in setup_packages
        common_dict = requirements_check()
      File "/tmp/pip-req-build-jnuak6qv/setup.py", line 224, in requirements_check
        stdout=subprocess.PIPE, stderr=subprocess.PIPE)
      File "/tmp/pip-req-build-jnuak6qv/setup.py", line 70, in run_command
        raise RuntimeError(msg)
    RuntimeError: command = python -c 'from __future__ import print_function; import sys; print(sys.executable)' failed with stdout = b'' stderr = b'/bin/sh: 1: python: not found\n' status 127

I also would like to install Corrfunc through pip, since it is listed as a requirement of another package, and I would like to resolve the dependencies automatically. What I however not fully understand: Since make is actually called from setup.py, the PYTHON environment variable could actually set in setup.py from sys.exec, instead of just crosschecking it.

lgarrison commented 4 years ago

I think this is actually failing because we're trying to do a consistency check between python specified in the Makefile and the python invoking setup.py, but in this case the python in the Makefile doesn't exist in your environment. I think the consistency check is probably overkill and we would always prefer to use the python from setup.py; i.e. sys.executable.

manodeep commented 4 years ago

@olebole What you say is exactly what is being done. The PYTHON in the Makefile is updated from setup.py; it must be that that line is not executing correctly (perhaps, because of pip involvement). Can you post the contents of common.mk from that temporary pip directory?

olebole commented 4 years ago

config.mk now has the right python path, like PYTHON:=/home/oles/test-venv/bin/python3, but it still fails.

lgarrison commented 4 years ago

Hey @manodeep, the problem (or at least one of them) is this line in setup.py where we are trying to do a consistency check between python specified in the Makefile and the python invoking setup.py:

https://github.com/manodeep/Corrfunc/blob/master/setup.py#L222-L224

But the Makefile python doesn't exist! It has yet to be replaced with the user's Python; during this check, it's literally just the python command. As I mentioned before, I advocate for skipping this check and just using the user's sys.executable out of the gate.

manodeep commented 4 years ago

Ahh I understand now. From setup.py, we should simply replace the PYTHON key in common.mk first with sys.executable before reading in the common.mk. We don't really need the make_python checks afterwards then.

manodeep commented 4 years ago

@olebole WIll you please check again if the latest updates fix the issue?

olebole commented 4 years ago

Yes, it works fine now. Thank you very much!

manodeep commented 4 years ago

Thanks @olebole. We will make the release shortly

manodeep commented 4 years ago

@olebole Corrfunc v2.3.3 is now out on PyPI - please let us know if this issue is resolved

olebole commented 4 years ago

Looks good to me -- thank you so much for your efforts! Everything is fine now.