GNU bug report logs - #72027
[PATCH] gnu: Add whisper-cpp.

Previous Next

Full log

Message #26 received at 72027 <at> debbugs.gnu.org (full text, mbox):

From: Juliana Sims <juli <at> incana.org>
To: atai <at> atai.org
Cc: 72027 <at> debbugs.gnu.org
Subject: Re: [PATCH v6] gnu: Add whisper-cpp.
Date: Fri, 23 Aug 2024 12:05:58 -0400

Well, I know I just asked if this was ready for review, but I figured 
I'd go ahead and do some "low-hanging fruit review" while I wait for 
you to get a chance to respond ;)

Fortunately, most of what I've noticed so far is minor and stylistic.  
There are some bigger, discussion-opening comments near the end.

Firstly, TODO comments are conventionally spelled as one word.  This is 
not grammatically correct, nor is it a requirement, but it is the 
convention and folks searching files for such comments will likely 
search for "TODO" rather than "TO DO".

Secondly, the synopsis should probably either drop the "Port of" or 
change "in" to "to".  I don't think this is a major issue; it just 
flows better like that imo.  I wouldn't oppose merging over this so if 
you prefer it the way it is, that's fine.

Next, the description wants expansion.  It should be three to five 
sentences.  The sentence there now is a fine start, though it needs 
ending punctuation.  Perhaps a sentence explaining how this package can 
be used ("whisper-cpp can be integrated with other programs to provide 
speech-to-text support" or something) and one explaining what makes it 
unique ("because whisper-cpp uses a leading speech recognition model, 
it is able to perform speech recognition rapidly with relatively few 
resources").  You could perhaps mention that it can run on CPU as well 
as GPU, that it offers some integrations with certain hardware, or so 
on.  Whatever you think is important or interesting; these are just 
some ideas :)

Now we enter the discussion-launching comments.

I notice that ggml is vendored.  This one is tricky.  Firstly, there is 
no standalone ggml package (yet; I saw your patch 65284).  Usually, 
Guix only asks that dependencies be unvendored if there is already a 
standalone package for the dependency, so unless and until that is 
merged there is no real issue.  Secondly, though, and this touches on 
the ggml patches, it seems that there are no formal releases of ggml 
yet, and development is happening in the repositories for whisper-cpp 
and llama-cpp as well as its own repository.  It seems these three 
versions of ggml are all slightly out of sync with each other.  It 
would be nice if upstream used git submodules to ensure their work was 
synchronized.  Alas, we as Guix can't do anything about that, and we 
must ensure the packages we offer work correctly.  The inconsistencies 
between these versions of ggml make me think packaging it separately 
would risk breakage.  With that in mind, might it be best to drop the 
standalone ggml patchset and just let llama-cpp and whisper-cpp vendor 
their versions?  While suboptimal because it results in building "the 
same package" multiple times, I would argue that the divergence in the 
code means they are not, in fact, the same package.

Finally, is this package complete?  Looking at the store directory for 
the package, I see headers and the like but no actual models.  Is this 
sufficient for using the inference?  Are client libraries or programs 
supposed to install models themselves?  Or can this package be used to 
generate models as described in the project's README?  If not, should 
it be able to?  I am admittedly fairly ignorant about the machine 
learning ecosystem, so feel free to explain as much as you think may be 
necessary.  My goal in these questions is ensuring users get what they 
expect from this package.

Relatedly, if this package is complete but requires further setup, I 
would strongly support explaining that in the package description.  As 
a user, I've encountered a few packages that require more setup and 
don't mention that they do, and I'm then frustrated and confused when I 
learn this from trying to use the package and then have no support from 
Guix in trying to make things work properly.  (Another step beyond this 
may be offering a system service which performs configuration, but that 
can be a future, separate patch.)

To circle back to what I mentioned in my first email, I would like to 
package the whisper.el Emacs mode[1].  Currently, whisper.el plans to 
install and compile whisper.cpp and its models itself; I think we as 
Guix should make this unnecessary for an imagined future emacs-whisper 
package.

Looking forward to hearing from you,
Juli

[1] https://github.com/natrys/whisper.el

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.