ServiceNow / build-pynini-wheels

Build `manylinux2014_x86_64` Python wheels for `pynini`, wrapping all its dependencies. This is a ServiceNow Research project that was started at Element AI.
Other
6 stars 7 forks source link

Adds test support. #4

Closed kylebgorman closed 3 years ago

kylebgorman commented 3 years ago

This draft is based on https://github.com/ElementAI/build-pynini-wheels/pull/3 from @daanzu, but re-enables the tests and also does some Bash style cleanup.

This goes to completion---wheels generated, all tests passing---on my Ubuntu PC in my office.

kylebgorman commented 3 years ago

Paging @PierreAndreNoel...I can replicate this now!

PierreAndreNoel commented 3 years ago

In other news, I can also reproduce this (OSX). So I'll approve and merge when/if the copyright issue is resolved.

kylebgorman commented 3 years ago

Any objection to me uploading the wheels in the meantime? (This seems to me just a question of whether we can redistribute the Dockerfile itself.)

PierreAndreNoel commented 3 years ago

No particular objection on our end to distribute the python wheels.

On a related note, we could decouple the problems of building up-to-date wheels (by putting back the copyright etc lines and merging this PR) from the problem of integrating this code to pynini's codebase (which will likely involve the CLA).

kylebgorman commented 3 years ago

I'll do that (to both points).

K

On Thu, Jul 15, 2021 at 1:11 PM Pierre-Andre Noel @.***> wrote:

No particular objection on our end to distribute the python wheels.

On a related note, we could decouple the problems of building up-to-date wheels (by putting back the copyright etc lines and merging this PR) from the problem of integrating this code to pynini's codebase (which will likely involve the CLA).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ElementAI/build-pynini-wheels/pull/4#issuecomment-880871220, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABG4OISXIYGB6YRZIHJ6XTTX4JDXANCNFSM5ABT6FAQ .

kylebgorman commented 3 years ago

PTAL and merge if this is acceptable---I just restored the copyright.

kylebgorman commented 3 years ago

I have this ready but PyPI upload requires a satisfactory resolution to this issue.

daanzu commented 3 years ago

@kylebgorman You may be able to get around that problem by stripping out all of the debug symbols, particularly for OpenFST. Something like find openfst/lib -name *${CMAKE_SHARED_LIBRARY_SUFFIX} | xargs strip Doing this brings https://github.com/daanzu/kaldi-active-grammar wheels down from ~200MB to ~15MB.

kylebgorman commented 3 years ago

@daanzu do you want to experiment with this? I'm still enough of a Docker dummy that I'm not 100% sure how to incorporate this into the procedure.

daanzu commented 3 years ago

@kylebgorman Sure, I will take a look.