Hopefully fixes builds on 3.12. #70

enzbus · 2023-10-07T09:16:47Z

Turns out extra numpy.distutils.get_info args were superflous, at least for plain builds.

Turns out extra numpy.distutils.get_info args were superflous, at least for plain builds.

bodono · 2023-10-07T13:36:25Z

Thanks for doing this Enzo! Does this still pick up the right blas / lapack libraries to link against?

enzbus · 2023-10-07T14:03:24Z

No :( I had a bug in my local compilation; I also tried to copy-paste the output of sys.info under python 3.11 and use it in python 3.12 but it didn't work. I suspect it's better to just rewrite the whole thing. (On another note, I noticed that the MacOS pypi package of scs was slower than the one I compiled on the test suite, so maybe the linkage was broken already.)On 7 Oct 2023, at 17:36, bodono ***@***.***> wrote: Reopened #70. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you modified the open/close state.Message ID: ***@***.***>

enzbus · 2023-10-07T14:51:59Z

Confirmed, linking was broken already. On python 3.11, clean environment.

pip install scs
otool -L env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so

gives:

env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so:
	@loader_path/scs/.dylibs/libopenblasp-r0.3.21.dylib (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.100.1)

While if I compile from source (inside scs-python)

pip install .
otool -L env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so

gives

env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so:
	/System/Library/Frameworks/Accelerate.framework/Versions/A/Accelerate (compatibility version 1.0.0, current version 4.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)

on Mac amd64.

rgommers

Looks pretty close already

meson.build

rgommers · 2023-10-08T09:35:21Z

meson.build

+'''
+    ],
+    check: true
+  ).stdout().strip()


This is the one annoying thing you have to do indeed - I will get around to replacing all this with a simple dependency('numpy') at some point.

For now, I'd recommend to include the relpath bit from SciPy: https://github.com/scipy/scipy/blob/166e1f2b1ea0a1a2c3d7b030bd829549f8a5844a/scipy/meson.build#L54-L57

That will ensure things work fine with an in-tree virtual environment (so numpy is inside your source tree from Meson's perspective).

Ok, I guess fixed (there are more try/except there that I don't understand).

bodono · 2023-10-08T10:01:28Z

Confirmed, linking was broken already. On python 3.11, clean environment.

pip install scs
otool -L env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so

gives:

env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so:
	@loader_path/scs/.dylibs/libopenblasp-r0.3.21.dylib (compatibility version 0.0.0, current version 0.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.100.1)

While if I compile from source (inside scs-python)

pip install .
otool -L env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so

gives

env/lib/python3.11/site-packages/_scs_indirect.cpython-311-darwin.so:
	/System/Library/Frameworks/Accelerate.framework/Versions/A/Accelerate (compatibility version 1.0.0, current version 4.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1311.100.3)

on Mac amd64.

Yes I also noticed that SCS was much slower from pip than when compiling locally, but I haven't had the bandwidth to investigate it unfortunately!

bodono · 2023-10-08T10:02:35Z

Thanks again for looking at this @enzbus and @rgommers ! Fingers crossed this works and can replace the very hacky code that was there before.

enzbus · 2023-10-08T10:39:33Z

@bodono @rgommers, current status: Basic build works, but there's no linking to blas in the compiled, so I'm probably not giving the right argument to py.extension_module. Also, right now pip installs _scs_direct.so but not the scs/__init__.py that is required to use it. Any hint?

rgommers · 2023-10-08T11:01:51Z

I'm just working on BLAS/LAPACK improvements in NumPy, so I can propose something. Rather than make it complicated now with 64-bit integer support (which seems incomplete?), the new Accelerate version for macOS >=13.3 (see note here), etc., maybe do only the basic setup first. Once NumPy has shipped the fancy version (auto-detect every option, full ILP64, switches for order of preference of known libraries, etc.) it can then be updated to mirror NumPy.

Sounds like you want regular 32-bit Accelerate on macOS preferably? And OpenBLAS or MKL elsewhere?

rgommers · 2023-10-08T11:04:14Z

but not the scs/__init__.py that is required to use it

You are missing a py.install_sources() to specify what to install - see at the bottom of numpy/scipy's meson.build files for examples. Alternatively, if you have a subdir with no compiled code and you want to install everything in that subdir, you can use install_subdir().

…o in dependency object)

enzbus · 2023-10-08T11:31:43Z

I'm just working on BLAS/LAPACK improvements in NumPy, so I can propose something. Rather than make it complicated now with 64-bit integer support (which seems incomplete?), the new Accelerate version for macOS >=13.3 (see note here), etc., maybe do only the basic setup first. Once NumPy has shipped the fancy version (auto-detect every option, full ILP64, switches for order of preference of known libraries, etc.) it can then be updated to mirror NumPy.

Sounds like you want regular 32-bit Accelerate on macOS preferably? And OpenBLAS or MKL elsewhere?

That would be great, and I guess it can be done by if/else on dependency.found(). I'm just trying to get a basic build now, and it's not linking BLAS after giving this output to meson compile

Program python3 found: YES (/usr/local/opt/[email protected]/bin/python3.11)
Library openblas found: NO
Library blas found: YES
Library cblas found: YES

Thoughts? And thanks!!

rgommers · 2023-10-08T12:51:46Z

I just tried meson setup build in my default SciPy dev env and got:

...
Program python3 found: YES (/home/rgommers/mambaforge/envs/scipy-dev/bin/python3.10)
Library openblas found: YES
Found pkg-config: /home/rgommers/mambaforge/envs/scipy-dev/bin/pkg-config (0.29.2)
Build targets in project: 2

You probably don't have pkg-config installed? What's happening is that for dependency('pkgname'), when Meson isn't aware of and has special handling for pkgname, it queries first pkg-config and then CMake to detect the dependency.

The better way to control this is via the `-Dbuildtype=` Meson build option. The default for meson-python is `release`, which means debug=False, optimization=3

enzbus · 2023-11-02T09:14:13Z

@bodono : status. Blas is linked statically when building on github CI (via a meson option passed to the command line). Still no luck for windows, but I think we're super close. If someone has a windows development machine they can probably fix it easily, see lines 20-31 of meson.build. If I were you I would start shipping Linux/Mac packages (or at least the source distribution), for example by:

Here's how you do it. Install brew (or whatever, to get a python3.12 -> https://docs.brew.sh/Installation)

brew install [email protected]
git clone https://github.com/enzbus/scs-python
cd scs-python
git submodule update --init
python3.12 -m venv env
source env/bin/activate
pip install .
pip install pytest
pytest
pip install build
python -m build
pip install twine
twine check dist/*
twine upload --skip-existing dist/*

enzbus · 2023-11-02T10:32:14Z

It builds and tests on windows!!! @rgommers have a look, we're using the same technique @bodono devised , with a few meson tweaks :)

bodono · 2023-11-02T10:56:54Z

Amazing work @enzbus ! Thanks again for doing this, drinks on me next time you're in London!

rgommers

Great work! This looks quite good to me - just one comment to help users a bit more when they are building from source and something goes wrong in BLAS detection.

rgommers · 2023-11-02T11:12:26Z

meson.build

+endif
+
+# We find anaconda blas on windows for github CI
+if not blas_deps[0].found()


This is specific to the CI setup for building Windows wheels, and won't work in case users attempt to build from source on Windows for some reason. That may or may not matter to you, but it's probably appropriate to at least emit a warning with message("explain-that-blas-wasn't-found-and-this-is-for-CI-only") here?

Also, you may want to change this line to

if host_machine.system() == 'windows' and not blas_deps[0].found() .... else error('OpenBLAS or Netlib BLAS/CBLAS is required on all platforms except Windows, and was not found.')

OK, I'm thinking of refactoring it: passing anaconda lib folder location as a meson option in the CI file, so the meson.build is not tailored for the compilation process on github.

That makes sense to me. You can do it as a CLI option, or you can craft an openblas.pc file which points to the right paths in a CI job and then add the directory of that .pc file to PKG_CONFIG_PATH. For the latter approach, see http://scipy.github.io/devdocs/building/blas_lapack.html#using-pkg-config-to-detect-libraries-in-a-nonstandard-location.

rgommers · 2023-11-02T11:12:59Z

meson.build

+  error('Missing the `scs_source` submodule! Run `git submodule update --init` to fix this.')
+endif
+
+# This uses the path as is, and avoids running the interpreter.


Minor: this comment is a left-over and doesn't apply, so best to clean it up.

rgommers · 2023-11-02T11:13:27Z

meson.build

+c_args = cc.get_supported_arguments('-Wno-unused-result')
+
+# meson doesn't support wildcards 
+# https://mesonbuild.com/FAQ.html#why-cant-i-specify-target-files-with-a-wildcard


Probably can be cleaned up as well now?

bodono · 2023-11-02T16:03:48Z

It looks like the github runners are no longer able to schedule on macos-10.15, I think we can change that to macos-latest, which should schedule easier and hopefully run successfully now. I can do it in a PR that you can merge, or you can do it here if you prefer.

enzbus · 2023-11-03T06:54:24Z

It looks like the github runners are no longer able to schedule on macos-10.15, I think we can change that to macos-latest, which should schedule easier and hopefully run successfully now. I can do it in a PR that you can merge, or you can do it here if you prefer.

OK, that explains why the MacOS jobs weren't running. As you prefer, I have limited time but would like to clean it a bit (it doesn't seem urgent, nobody replied on cvxpy/cvxpy#2269 ). Ideally we can also migrate the optional builds (openmp, cuda) to meson and reduce the size of the github yaml file. (We only need conda for installing openblas on windows, etc.) Just a note, meson doesn't support python <= 3.7, so if we eliminate the old setup.py we'll have to drop python 3.7 (but the wheels and source dists will remain on PyPI).

rgommers · 2023-11-03T09:49:49Z

Just a note, meson doesn't support python <= 3.7, so if we eliminate the old setup.py we'll have to drop python 3.7 (but the wheels and source dists will remain on PyPI).

In practice that makes it harder to do Python 3.5-3.6, but not impossible. Meson by design does not depend on it being implemented in Python, it has its own DSL and you invoke it via a meson executable that's on PATH. So it is possible to install Meson in, say, a Python 3.11 env, prepend its bin directory to PATH, and then use it in a non-isolated build from an env with an older Python version.

That said, meson-python is tied to Python version too (and is at >=3.7), and avoiding that constraint is much harder. So in practice, yes, >=3.7 is the limit now.

…fail)

…s that were there originally

bodono · 2023-11-03T13:21:05Z

All checks have passed! I will merge as soon as you let me know it's ready @enzbus .

enzbus · 2023-11-03T13:36:04Z

Ok, @bodono, I approve too. Please go on and merge, hopefully there won't be issues when the final build and upload is triggered.

Hopefully fixes builds on 3.12.

352fe5f

Turns out extra numpy.distutils.get_info args were superflous, at least for plain builds.

enzbus mentioned this pull request Oct 7, 2023

Python 3.12 compatibility #69

Closed

enzbus added 2 commits October 7, 2023 09:19

Update Makefile

752a8d6

Update Makefile

7225748

enzbus closed this Oct 7, 2023

bodono reopened this Oct 7, 2023

enzbus added 5 commits October 8, 2023 12:38

trying to switch to meson

ae09b24

Merge branch 'master' of github.com:enzbus/scs-python

a8dc9ee

trying to get a meson build

73946fb

Makefile superflous

951ce78

moved vendored scs folder to avoid naming conflict

a7d0868

rgommers reviewed Oct 8, 2023

View reviewed changes

enzbus added 2 commits October 8, 2023 14:11

basic build working

5895b15

@rgommers comments, macros, and trying blas link

6672832

added indirect

b5a3641

enzbus added 3 commits October 8, 2023 15:07

indirect compiles (was missing flags)

e88be4f

pip installation works, including __init__.py

c2f36be

pip installatin works, only blas/lapack linking missing (probably typ…

ae00033

…o in dependency object)

Error out with a clear message if the SCS git submodule is missing

d29c051

Remove -O3; that's the default in meson-python, and isn't portable.

7601bc6

The better way to control this is via the `-Dbuildtype=` Meson build option. The default for meson-python is `release`, which means debug=False, optimization=3

enzbus added 7 commits November 2, 2023 11:31

numpy.show_config changed api

5c3d16c

typo

72140e5

one more try

580477d

miniconda lib location for windows on github ci

ef655a2

typo

ee9017b

maybe inverted slash?

538b995

maybe inverted slash?

a859327

enzbus added 3 commits November 2, 2023 13:17

with same mixed spelling returned by np.show_config

e59e496

some debugging information

bcacd76

it was openblas.lib!

f23e144

bodono approved these changes Nov 2, 2023

View reviewed changes

rgommers approved these changes Nov 2, 2023

View reviewed changes

enzbus added 8 commits November 3, 2023 16:15

fixed @rgommers comments on meson.build

6d70a6b

switched from macos-10.15 to macos-latest in github ci

4b081e6

renamed OLD_setup.py to legacy_setup.py

b893196

if clause wasnt right

12d45d4

removed bunch of conda installs that should not be needed (revert if …

f7e2e98

…fail)

typo

a0fb593

we needed numpy

6036646

problems with conda env and pip, better to restore same conda install…

9b7bbc1

…s that were there originally

bodono merged commit a72909b into bodono:master Nov 3, 2023
47 checks passed

h-vetinari mentioned this pull request Jan 24, 2024

Update to 3.2.4 conda-forge/scs-feedstock#38

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hopefully fixes builds on 3.12. #70

Hopefully fixes builds on 3.12. #70

enzbus commented Oct 7, 2023

bodono commented Oct 7, 2023

enzbus commented Oct 7, 2023 via email

enzbus commented Oct 7, 2023 •

edited

Loading

rgommers left a comment

rgommers Oct 8, 2023

enzbus Oct 8, 2023

bodono commented Oct 8, 2023

bodono commented Oct 8, 2023

enzbus commented Oct 8, 2023

rgommers commented Oct 8, 2023 •

edited

Loading

rgommers commented Oct 8, 2023

enzbus commented Oct 8, 2023

rgommers commented Oct 8, 2023

enzbus commented Nov 2, 2023

enzbus commented Nov 2, 2023

bodono commented Nov 2, 2023

rgommers left a comment

rgommers Nov 2, 2023

enzbus Nov 3, 2023

rgommers Nov 3, 2023

rgommers Nov 2, 2023

rgommers Nov 2, 2023

bodono commented Nov 2, 2023

enzbus commented Nov 3, 2023

rgommers commented Nov 3, 2023

bodono commented Nov 3, 2023

enzbus commented Nov 3, 2023

Hopefully fixes builds on 3.12. #70

Hopefully fixes builds on 3.12. #70

Conversation

enzbus commented Oct 7, 2023

bodono commented Oct 7, 2023

enzbus commented Oct 7, 2023 via email

enzbus commented Oct 7, 2023 • edited Loading

rgommers left a comment

Choose a reason for hiding this comment

rgommers Oct 8, 2023

Choose a reason for hiding this comment

enzbus Oct 8, 2023

Choose a reason for hiding this comment

bodono commented Oct 8, 2023

bodono commented Oct 8, 2023

enzbus commented Oct 8, 2023

rgommers commented Oct 8, 2023 • edited Loading

rgommers commented Oct 8, 2023

enzbus commented Oct 8, 2023

rgommers commented Oct 8, 2023

enzbus commented Nov 2, 2023

enzbus commented Nov 2, 2023

bodono commented Nov 2, 2023

rgommers left a comment

Choose a reason for hiding this comment

rgommers Nov 2, 2023

Choose a reason for hiding this comment

enzbus Nov 3, 2023

Choose a reason for hiding this comment

rgommers Nov 3, 2023

Choose a reason for hiding this comment

rgommers Nov 2, 2023

Choose a reason for hiding this comment

rgommers Nov 2, 2023

Choose a reason for hiding this comment

bodono commented Nov 2, 2023

enzbus commented Nov 3, 2023

rgommers commented Nov 3, 2023

bodono commented Nov 3, 2023

enzbus commented Nov 3, 2023

enzbus commented Oct 7, 2023 •

edited

Loading

rgommers commented Oct 8, 2023 •

edited

Loading