Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
{lib}[GCCcore/12.3.0] PyTorch-bundle v2.1.2, SoX v14.4.2, libmad v0.15.1b, ... w/ CUDA 12.1.1 #19987
{lib}[GCCcore/12.3.0] PyTorch-bundle v2.1.2, SoX v14.4.2, libmad v0.15.1b, ... w/ CUDA 12.1.1 #19987
Changes from 5 commits
9f5f0b7
569fac8
438f8ed
576c988
b2087ca
9d1901e
8e26812
3a9dbc4
ebf12e8
d7bf83a
c72f7bd
aa392e8
b24b8d3
2efa625
e11c706
3efccd7
71131fe
7016977
be7f488
6d1564a
cd5b136
b30801e
9709e77
808d8b0
7febf35
ed8bfb3
a485746
3e6fd23
df05212
e1056b4
f7043e7
89113fa
645b07c
4f26d28
9022547
1db37ca
bb95f51
46662ec
d33f65f
aad5993
e58d908
1397cb8
9e737f4
77f29fb
969579d
959eef6
e36468a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SentencePiece requires protobuf/24.0 as a dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand how only you managed to trigger this. Also, i assume you meant protobuf-python/4.24.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I mean
It uses pkg-config to search for protobuf-lite and fails without it.
It fails for me since I don't have stuff like that in my container :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
protobuf-lite is included in sentencepiece as a thirdparty package (maybe we don't actually want that, but regardless), i would have expected it to find that, rather than having the python module link to something potentially different.
And surely so does noone else on their OS either. I certainly don't have it, no trace of protobuf, protobuf-lite.
After some digging, I found that one can use external packages for Abseil and protobuf, using
when attempting this, I also noticed that
'prebuildopts': 'export PKG_CONFIG_PATH=%(installdir)s/lib/pkgconfig:$PKG_CONFIG_PATH
my pkgconfig stuff ended up in lib64, not lib. Fixing that made
sentencepiece
(the python part) not even look for protobuf. It shouldn't be looking for that stuff to start with, since it should just be using the already compiledSentencePiece
from the first component. So, I think we have some lib/lib64 difference here.I attempted to use external protobuf and Abseil, but I got very very confusing problems. the ABSL_FLAG parsing completely broke, with strange broken output and warnings messages and strange output from even basic things like
spm_train --version
. I couldn't make any sense of it, so I reverted back.I've added pkgconf as a builddep and changed the PKG_CONFIG_PATH to include lib64 instead, which I hope will be correct for everyone? I'm not sure. Mixing in a different external protobuf is no good, because it seems to conflict with the bundled thirdparty abseil, and taking in both of them just broke things for me.