Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add OPENBLAS_NO_F2C math library option #1215

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johngebbie
Copy link
Contributor

@johngebbie johngebbie commented Dec 3, 2022

This patch allows building with lapack instead of clapack.
Without this it fails to link f2c which lapack does not have or need.
I have also made a pull request for your kaldi: alphacep/kaldi#5

This helps on distros like Void Linux where there is a lapack package but not a clapack package.

@nshmyrev
Copy link
Collaborator

nshmyrev commented Dec 3, 2022

Thanks. This is just called OPENBLAS in kaldi right? And also requires libgfortran, isn't it?

@johngebbie
Copy link
Contributor Author

johngebbie commented Dec 3, 2022

Ah thank you, kaldi seems to be compiling well with OPENBLAS so that does make the kaldi pull request redundant.

There is a comment saying libgfortan.so.3 is no longer required but it's a dependency of the lapack package anyway so I can't confirm that.

Would you like me to rename it from OPENBLAS_LAPACK to OPENBLAS in this pull request?

@nshmyrev
Copy link
Collaborator

nshmyrev commented Dec 3, 2022

Yes please, rename to just OPENBLAS. And libgfortran is still needed, you can check with ldd. Some linkers don't complain, some will.

@johngebbie johngebbie changed the title Add OPENBLAS_LAPACK math library option Add OPENBLAS math library option Dec 3, 2022
@johngebbie
Copy link
Contributor Author

johngebbie commented Dec 3, 2022

Okay I renamed it.

Kaldi (and then vosk) compiled with --mathlib=OPENBLAS but the vosk check failed with:

/usr/bin/ld: ../src/libvosk.so: undefined reference to '_gfortran_concat_string'

So using it vosk must need linked to the gfortran library.

Is it surprising that my kaldi pull request works with lapack but doesn't need linked to libgfortan?
The new option it adds is just like OPENBLAS_CLAPACK but with f2c removed.
Should I reopen that pull request?
I think I will use both patches as they seem to be an improvement.

@nshmyrev
Copy link
Collaborator

nshmyrev commented Dec 3, 2022

_gfortran_concat_string is part of libgfortran, not f2c

@johngebbie
Copy link
Contributor Author

I'm going to bed. Please don't merge this yet.

I feel this is maybe different again from kaldi's OPENBLAS and so maybe should be named differently.

_gfortran_concat_string is part of libgfortran, not f2c

Yeah. Do you think my patches are useful as they don't require libgfortran but use lapack not clapck?

@johngebbie johngebbie changed the title Add OPENBLAS math library option Add OPENBLAS_NO_F2C math library option Dec 4, 2022
@johngebbie
Copy link
Contributor Author

johngebbie commented Dec 4, 2022

I have renamed the option OPENBLAS_NO_F2C as it different from kaldi's OPENBLAS that uses libgfortran.
I will be happy for both pull requests to be merged once the continuous integration has finished successfully.

@johngebbie
Copy link
Contributor Author

The continuous integration finished successfully (see tests at bottom of void-linux/void-packages#39015). I'm happy for this and the kaldi pull request to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants