bigmlcom / sensenet

0 stars 8 forks source link

Building Wheels Automatically #33

Closed charleslparker closed 2 years ago

charleslparker commented 2 years ago

I'm trying to use Github's integration with CIBuildWheel to build wheels for all platforms automagically. Let's see how we do.

charleslparker commented 2 years ago

Good news / bad news on creating the pre-built python wheels for sensenet:

Good news: GitHub is automatically able to create nice builds for the wheels on python 3.8, 3.9, and 3.10, for both x86_64 and Apple Silicon running mac os, and for the same Pythons on manylinux2014

Bad news: Windows support for building tensorflow extensions is at best second-class (see, e.g., https://github.com/tensorflow/models/issues/2511) and I can't figure out a quick way to make it work, nor does tensorflow support either 32-bit or i686 hardware at all.

For windows, I've turned off building the custom extension, which should at least allow installation without complaint. Most deepnets (even most image deepnets) should still work fine, but some (specifically, the ones with tree_embedding=True will throw an exception when you try to instantiate them. @mmerce - You are only using TF for image models, yes?

/cc @unmonoqueteclea

unmonoqueteclea commented 2 years ago

Great news! ARM64 wheels will also allow easy installation on Raspberry PIs and similar devices!

mmerce commented 2 years ago

For windows, I've turned off building the custom extension, which should at least allow installation without complaint. Most deepnets (even most image deepnets) should still work fine, but some (specifically, the ones with tree_embedding=True will throw an exception when you try to instantiate them. @mmerce - You are only using TF for image models, yes?

I'm afraid we use sensenet for the local deepnet predictions, whatever types their fields are.

charleslparker commented 2 years ago

I'm afraid we use sensenet for the local deepnet predictions, whatever types their fields are.

In theory we could fall back to the existing deepnet prediction code in cases where we could not install the extension

mmerce commented 2 years ago

I'm afraid we use sensenet for the local deepnet predictions, whatever types their fields are.

In theory we could fall back to the existing deepnet prediction code in cases where we could not install the extension

Yes, we could recover the code in the previous versions and use that if no sensenet is found...

charleslparker commented 2 years ago

Yes, we could recover the code in the previous versions and use that if no sensenet is found...

Just to clarify - the extension is only used in a minority of cases, so the library will still work most of the time even when the extension cannot be compiled. Another option here is for me to write a pure tensorflow equivalent of the extension to use in the case where it can't be built. I've already done this and can resurrect the code, the only thing is that it's much slower than the compiled extension (which is why I wrote it in the first place). But maybe that is better (as it's less to maintain) than falling back to the old code?

There's still the case where tensorflow is unsupported on the given platform . . .

mmerce commented 2 years ago

There's still the case where tensorflow is unsupported on the given platform . . .

charleslparker commented 2 years ago

If tensorflow is supported but we cannot compile the extension: Is the tree_embedded=True attribute only going to appear when the dataset contains images (and for some specific cases of those)?

It's actually the opposite. tree_embedding = True is something that happens in a lot of old models. It can still happen in new models, including but not limited to image models, but it's not the default anymore so it's much less likely. Thinking about it more, it seems to me that resurrecting the less efficient code may be a good idea. It will only go slow for windows users, who are used to things going slow anyway, and it will at least not complain and do everything it should.

charleslparker commented 2 years ago

@mmerce - I think we have a reasonable plan here. Would you mind approving this PR and I can work on the fix for the tree problem? This by itself will still be a significant improvement for non-windows users.

@unmonoqueteclea - FYI for some reason related to h5py, linux ARM64 wheels will only build for python 3.8 and 3.9, but not 3.10.

mmerce commented 2 years ago

@mmerce - I think we have a reasonable plan here. Would you mind approving this PR and I can work on the fix for the tree problem? This by itself will still be a significant improvement for non-windows users.

Sure, but let me see if I understand the plan. If tensorflow is not supported or the model has tree_embedded=True, then we use the old code and we use sensenet otherwise?

charleslparker commented 2 years ago

If tensorflow is not supported or the model has tree_embedded=True, then we use the old code and we use sensenet otherwise?

If tensorflow is not supported, you can fall back to the old code (or show an error), but in all other cases sensenet will try to make the prediction. I will do something sensible the tree_embedding for windows in sensenet.