search

mehta-lab / waveorder

Wave optical models and inverse algorithms for label-agnostic imaging of density & orientation.
BSD 3-Clause "New" or "Revised" License
15 stars 4 forks source link

`opencv-python` is taking >30 minutes to build on M1 #107

Closed talonchandler closed 1 year ago

talonchandler commented 1 year ago

opencv-python just released version 4.7.0.72 and it is taking >30 minutes to build on my M1. Installing the same version on Windows is fast, and installing the earlier version 4.7.0.68 is also fast.

@ziw-liu I'm considering pinning opencv-python==4.7.0.68 for this release, so that I can move forward. As you mention, we not depending on opencv very much and it may be causing a bug so we're planning to remove this dependency soon anyway.

@ziw-liu are you able to replicate an extremely slow opencv-python build? This problem affects recOrder installations on my M1 too.

talonchandler commented 1 year ago

Actually...just pinning and releasing in waveorder doesn't fix the issue since existing versions of recOrder will install opencv too. We may need to consider pinning the version in recOrder too.

talonchandler commented 1 year ago

The build just completed (roughly 35 minutes), and it seems to be working. I'll test more this afternoon.

As expected, the long build only needed to happen once, so subsequent reinstallations are fast.

I still need to test and think about the best plan...input welcome.

ziw-liu commented 1 year ago

We may need to consider pinning the version in recOrder too.

RecOrder at the main branch does not depend on OpenCV anymore (https://github.com/mehta-lab/recOrder/commit/e68aaaba46383a0a2aed6a87a20dad1099487ddd). Since recOrder 0.2.x pins waveorder to 1.0.0rc0 anyway and 0.3.0 will be released without the OpenCV dependency, I guess this is not a big issue?

ziw-liu commented 1 year ago

That being said, I think we should remove OpenCV for waveorder too. The only function that uses it is not tested, and confusingly written. I can make a quick fix or removal if that sounds good.

Edit: the function is used locally in the viewer, should be easy to test.

talonchandler commented 1 year ago

Okay great, thanks Ziwen. I'll go ahead and upload the release, since this is at worst an inconvenience.