Headers not included?

[h-vetinari]

Hey all

I'm trying to use kaldi in conda-forge/staged-recipes#17082, but it runs into

/home/conda/staged-recipes/build_artifacts/torchaudio_1637900895972/work/torchaudio/csrc/kaldi.cpp:2:10: fatal error: feat/pitch-functions.h: No such file or directory
     #include "feat/pitch-functions.h"
              ^~~~~~~~~~~~~~~~~~~~~~~~
    compilation terminated.

I tried to look through the recipe, but the patches are a bit intransparent to me, and the testing only checks for the binaries, not headers. Is there perhaps a reason why the headers aren't packaged? Is there the equivalent of a libkaldi somewhere? Would appreciate some help with this @conda-forge/kaldi

[mmcauliffe]

Yes for getting kaldi working initially I just included the binaries since that's what the Montreal Forced Aligner uses. It shouldn't be too hard to include the headers in the export, so I'll try to get that sorted soon.

…

On Thu., Nov. 25, 2021, 8:56 p.m. h-vetinari, ***@***.***> wrote: Hey all I'm trying to use kaldi in conda-forge/staged-recipes#17082 <conda-forge/staged-recipes#17082>, but it runs into /home/conda/staged-recipes/build_artifacts/torchaudio_1637900895972/work/torchaudio/csrc/kaldi.cpp:2:10: fatal error: feat/pitch-functions.h: No such file or directory #include "feat/pitch-functions.h" ^~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. I tried to look through the recipe, but the patches are a bit intransparent to me, and the testing only checks for the binaries, not headers. Is there perhaps a reason why the headers aren't packaged? Is there the equivalent of a libkaldi somewhere? Would appreciate some help with this @conda-forge/kaldi <https://github.com/orgs/conda-forge/teams/kaldi> — You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub <#15>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVJOT3KMIIVGVBNW2T2UCDUN4HQJANCNFSM5IZ2LJXQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[h-vetinari]

Any news on this @mmcauliffe? 🙃

[mmcauliffe]

Yeah so I have it mostly working, along with getting the CUDA aspects of Kaldi compiling as well, just running some tests (and going to make a PR for it in the Kaldi repo as well).

[mmcauliffe]

@h-vetinari Looking around at build failures with cuda, it looks like you've done a lot of work there. Is it possible to actually get it working on conda forge's infrastructure, or should I shelve the CUDA support for the time being?

[h-vetinari]

Yeah so I have it mostly working, along with getting the CUDA aspects of Kaldi compiling as well, just running some tests (and going to make a PR for it in the Kaldi repo as well).

Awesome, thanks a lot!

@h-vetinari Looking around at build failures with cuda, it looks like you've done a lot of work there. Is it possible to actually get it working on conda forge's infrastructure, or should I shelve the CUDA support for the time being?

Just saw that you opened #16, will try to have a look.

[mmcauliffe]

Thanks, appreciate you taking a look! I've been mostly looking over how the pytorch and torchvision feedstocks are set up for it.

[mmcauliffe]

@h-vetinari Looks like there's some bigger issues with the CUDA stuff here: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=423851&view=logs&j=4f922444-fdfe-5dcf-b824-02f86439ef14&t=b2a8456a-fb11-5506-ca32-5ccd32538dc0. Maybe something up with the CXX version, I saw you were on several threads for updating that, but it seems to be causing some sort of conflict here. I'll try to an intermediate release with the headers.

[mmcauliffe]

Merged in the PR with headers, and they're installed on my linux test. Let me know if there's anything more that you need there, and feel free to reopen!

[h-vetinari]

Thanks so much! Sorry I didn't get around to looking at the cuda stuff. I don't know anything about kaldi, much less kaldi+CUDA, but you could open an issue and document your findings there as a starting point for a PR at some later date?

[h-vetinari]

Unfortunately, the headers on OSX are still missing... Could you reopen this issue please?
We should add a test to make sure they're really there.

I just started looking at #16 now, and there's a bunch of things that could/should be improved (e.g. around lapack handling), but before I do that, I first would like to understand the need for such a gigantic patch relative to upstream? It makes it really hard for me what's going on - could you maybe share how you arrived at that, so I don't go on a wild goose chase trying to slim it down?

[mmcauliffe]

Sure, here's the PR that's open on Kaldi that has the changes included in the patch: kaldi-asr/kaldi#4669

I'll add some testing and look into why they aren't getting exported on mac when they are on other platforms.

[h-vetinari]

Thanks a lot. But they're not getting exported on linux either with the last build...

The header-test I was talking about would look like this:

test:
  commands:
    # headers
    {% for each_header in ["expected.h", "headers.h", "and_so_on.h"] %}
    - test -f $PREFIX/include//{{ each_header }} || (echo "{{ each_header }} not found" && exit 1)  # [unix]
    - if not exist %LIBRARY_INC%\\{{ each_header }} exit 1                                          # [win]
    {% endfor %}

[h-vetinari]

This would let you know if those headers have been installed in a way that conda actually knows about them.

[mmcauliffe]

Weird, somehow my test one did have headers. I've written up the test (in the python test file since there's 359 header files to test), testing it locally and then I'll push it to the current PR that's fixing some issues with the dylib linking.

Any pointers on optimizing the cmake and blas aspects would be very much appreciated, I do not usually work on this kind of thing.

[h-vetinari]

Ping @mmcauliffe

in the python test file since there's 359 header files to test

I think it would be fine to start with just that we know there are consumers for (e.g. feat/pitch-functions.h). It's quite likely that if the build works for one header, it works for the others as well.

I did something similar for nvidia/cutlass recently:

    # headers:
    # This list should be substantially longer, but upstream just globs
    # all *.h files. These are the headers required for triton, but
    # relevant headers can be added as needed/desired.
    {% for each_header in ["library/handle.h", "library/library.h",
                           "library/operation_table.h", "library/singleton.h"] %}
    - test -f $PREFIX/include/cutlass/{{ each_header }} || (echo "{{ each_header }} not found" && exit 1)  # [unix]
    - if not exist %LIBRARY_INC%\cutlass\{{ "\\".join(each_header.split("/")) }} exit 1                    # [win]
    {% endfor %}