Repository navigation
Conversation
Author
|
Hello @cjpais, I hope you are doing well. Please feel free to stop me at anytime if my PRs are becoming annoying to you. I'm not contributing to open source to get a job, but I face issues while working on my own projects, solving them, then trying to upstream the fixes in small focused PRs. Thanks! |
Contributor
|
Thanks for this, not annoying, appreciate it and I'll take a look and pull in when I have a chance |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Session init only rejects a negative
n_threadstoday. Any positive value is accepted, but values above ggml's thread limit (GGML_MAX_N_THREADS, 512) don't work: the CPU backend barely makes progress, or thread creation fails later intranscribe_runand is reported as out of memory.This PR adds the upper bound to the existing check, so session init returns
TRANSCRIBE_ERR_INVALID_ARGright away. The four session types (ASR, VAD, langid and diarize) each had the samen_threads < 0test, so the rule now lives in one small helper,valid_n_threads()in transcribe-abi.h, and the public headers document the range. Values from 0 to 512 behave exactly as before.Scope
valid_n_threads()helper.There is no ABI change. The struct layouts are the same, and the bindings pass the value through, so they get the new error as well.
AI Assistance
Yes. As in #228, I used an AI assistant to help build the code and the wording of this description. I reviewed the changes and ran the checks below.
Validation
transcribe-cliwith parakeet-tdt-0.6b-v3 Q8_0 on samples/jfk.wav,--backend cpu, on a 6 CPU machine:On this branch every rejected value returns within about 1.5 s, which is mostly the model load.
scripts/ci/clang-format.sh --check passes, and ctest passes (61 tests; the diarize CLI smoke is skipped without a model).
This only enforces ggml's hard limit. A count between the number of CPUs and 512 is still accepted, and it is still very slow when it oversubscribes the machine (512 on this machine did not finish in 120 s either). If you think it is useful, I can follow up with a warning when n_threads is above the number of usable CPUs.