SimpleCRF icon indicating copy to clipboard operation
SimpleCRF copied to clipboard

Workflows to produce binaries

Open andreped opened this issue 2 years ago • 9 comments

As discussed in this issue #7 there was a need for binaries to more easily install SimpleCRF on different platforms.

This was especially relevant for Windows which required VS and Visual C++ build tools to be able to install SimpleCRF as it needs to be built from scratch when installing.

I mostly focused on adding and improving the workflows, but I found it relevant to make some minor adjustments as well.

Lots of time (and commits) was spent in trying to get the demos to run on the cloud. However, the demos include plt.show() and when these are run on a headless machine it resulted in hangs (or so it seemed). Therefore, these unit tests I added were removed for now, to be added in the future I suppose (if of interest).

Changes:

  • Added workflows for Windows, Ubuntu Linux, and macOS for the Python versions 3.6-3.10
  • Updated the demos to use arguments instead of input(), which does not require user input (args necessary to run demos on the cloud with GitHub Action)
  • Modified setup.py file to include information on supported python versions
  • Minor refactoring

EDIT:

  • Removed Python 3.10 build for now, only supports 3.6-3.9 (as SimpleITK does not support 3.10 yet)
  • Also merged all workflows into a single workflow (ran as a nested matrix). Only one zip is produced with all wheels
  • Also added unit tests for all wheel builds, running all three demos
  • For the demos to work I had to swap the plt.show() with plt.savefig(), as these are run on a headless server

andreped avatar Feb 01 '22 16:02 andreped

Any updates on this?

SachidanandAlle avatar Apr 04 '22 10:04 SachidanandAlle

Any updates on this?

Not really up to me. But if we hear nothing from the owner, @taigw, I could make a temporary PyPI project called SimpleCRF-binaries, which has this fix. However, the easiest solution is just to accept this PR and add the binaries.

andreped avatar Apr 04 '22 10:04 andreped

i think lets go with plan-b.. it's been a while.. we don't see any chances to see this PR getting merged... may be once it's merged, we can start using regular simplecrf package from PIP...

@andreped thanks for helping on this.. we can create SimpleCRF-bin (or any other name that makes sense) as alternative package and MONAILabel will start using them for now..

SachidanandAlle avatar Apr 05 '22 09:04 SachidanandAlle

@SachidanandAlle I can make the PyPI repo, as I was the one making these edits. I can give you admin rights to the repo, if you'd like. I will use my fork as base.

andreped avatar Apr 05 '22 10:04 andreped

@SachidanandAlle I have created the temporary PyPI project: https://pypi.org/project/SimpleCRF-binaries/

To install simple run the command: pip install SimpleCRF-binaries

Let me know if there are any issues. As this is a temporary solution, I only made the minimal necessary edits to make this work.

andreped avatar Apr 05 '22 11:04 andreped

@andreped Is there a reason why the macOS whl for Python 3.9 is the only one that has minimum requirement of macOS 10.15 (SimpleCRF_binaries-0.2.1.1-cp39-cp39-macosx_10_15_x86_64.whl) compared to macOS 10.14 for the other whl files?

Also in 3D Slicer as of right now soon to be released Slicer 5.0 has minimum support for macOS 10.13. We are actually not using the latest numpy version because it bumped its minimum macOS whl version to macOS 10.14. See https://github.com/Slicer/Slicer/commit/81517ef9a6a4a4d69efce347adc3fce9d03b785e.

jamesobutler avatar Apr 05 '22 12:04 jamesobutler

@jamesobutler I believe this happened automatically.

AFAIK, if you build wheels using GitHub Actions, only macOS >= 10.15 is supported. It sucks because quite often 10.13-10.14 are used in production. But there is little I can do about it.

There is something called CMAKE_OSX_DEPLOYMENT_TARGET which you can set, at least for CMake, that solves this issue. However, I have tried to have a similar fix for Python projects, and I have not gotten it to work. There is also this answer from the GitHub team: this...

But maybe you have solved this somehow for MONAI Label, @SachidanandAlle, @masadcv? I could incorporate your fix to support more macOS versions. But yeah, I could do some tests at my side. However, it is a bit challenging, as I would have to build on the cloud and run tests on a local macOS 10.13 machine...

To produce the binaries for the new PyPI project, I am working on a separate branch: https://github.com/andreped/SimpleCRF/tree/binaries

If you find a fix, you can let me know where to add it.

andreped avatar Apr 05 '22 13:04 andreped

@jamesobutler I have tried to fix this in the latest release candidate now: pip install SimpleCRF-binaries==0.2.1.3rc1

At least I was able to install it and import "denseCRF" and "maxflow". You still need to upgrade numpy. But since MONAI Label has chosen to not use SimpleCRF in the future, due to licensing issue, I guess these fixes will only be relevant for the SimpleCRF community. These edits are in the binaries branch of my fork.

andreped avatar Apr 05 '22 17:04 andreped

@taigw Hello! Is it possible to merge this and make a new patch release such that when installing from PyPI you install through the binary wheels?

andreped avatar Aug 23 '22 14:08 andreped