cbrnr / sleepecg

Sleep stage detection using ECG
BSD 3-Clause "New" or "Revised" License
93 stars 25 forks source link

[JOSS Review] Installation fails on macOS #157

Closed richrobe closed 1 year ago

richrobe commented 1 year ago

(as part of the JOSS Review)

The straightforward installation of SleepECG fails on my system (Python 3.9 on 2017 Macbook Pro with macOS Ventura 13.3, Intel CPU, no GPU) using the current pyproject.toml version with the following error message: with the following error message:

Could not find a version that satisfies the requirement tensorflow-macos>=2.7.0; (sys_platform == "darwin" and python_version < "3.11") and extra == "full" (from sleepecg[full]) (from versions: none)

Replacing the optional dependencies tensorflow-macos and tensorflow-metal by tensorflow in the pyproject.toml file fixes the issue and correctly installs the package with all dependencies.

cbrnr commented 1 year ago

That's weird, I get the same (or at least similar error) on my Intel Mac (I only tested on my M2 Mac previously). My idea was to provide optimized builds/packages for Macs, meaning native ARM64 support (tensorflow-macos) as well as Metal support (tensorflow-macos) for 3.8 <= Python <= 3.11. The "official" tensorflow package does not support ARM64.

Do you know which requirements exactly cannot be satisfied? A tensorflow-macos wheel should be available for your platform (https://pypi.org/project/tensorflow-macos/#files). I get this error with tensorflow-metal even though a matching package seems to be available.

If this cannot be resolved, I'm happy to revert to standard tensorflow (although it really bugs me that this still does not support ARM64, but there will be a wheel in the next release). Maybe we should have separate TF dependencies for each architecture?

richrobe commented 1 year ago

I directly get an error when trying to manually install the mac-native tensorflow packages tensorflow-mac and tensorflow-metal via pip:

$ pip install tensorflow-macos
>>> ERROR: Could not find a version that satisfies the requirement tensorflow-macos (from versions: none)
>>> ERROR: No matching distribution found for tensorflow-macos
$ pip install tensorflow-metal
>>> ERROR: Could not find a version that satisfies the requirement tensorflow-metal (from versions: none)
>>> ERROR: No matching distribution found for tensorflow-metal

which is weird because, as you correctly pointed out, there should be a build available for my platform...

cbrnr commented 1 year ago

Strange. I think I tried installing just these packages yesterday, and it worked. Can you try and see if you can get more information out of pip with -vvv?

cbrnr commented 1 year ago

I can't really test this on my old Intel Mac, because I'm stuck with macOS 11.7.6. The problem is that the latest tensorflow-macos and tensorflow-metal versions require macOS 12, so unfortunately, I can't test this. I will check on my new ARM64 Mac though (I expect that everything works). Maybe the best solution is really to require tensorflow on Intel Macs and tensorflow-macos/tensorflow-metal on ARM64 Macs. I think it should be possible to specify these requirements in pyproject.toml.

cbrnr commented 1 year ago

OK, I'm pulling the plug on tensorflow-macos and tensorflow-metal in favor of the official tensorflow package. In addition to problems when installing these Apple-maintained packages, I also couldn't import one of our pre-trained models. In contrast, everything works fine with tensorflow (which BTW now has an ARM64 macOS package on PyPI anyway).

richrobe commented 1 year ago

Alright, that's of course fine for me! Thank you for your efforts 🙂

cbrnr commented 1 year ago

The only downside is that it doesn't run on the GPU then. But it should still be plenty fast, and if someone really wants GPU support they can try and use the Apple-provided packages at their own risk.