update readme #6
Conversation
Pull Request Review: README UpdateThank you for this comprehensive README overhaul! This is a significant improvement that transforms the documentation from a work-in-progress outline into production-ready documentation. Below is my detailed review: ✅ Strengths1. Excellent Structure & Organization
2. Strong Technical Content
3. User-Focused Documentation
|
| Category | Status | Count |
|---|---|---|
| Critical Issues | 🔴 | 4 |
| Medium Issues | 🟡 | 0 (included in critical) |
| Minor Issues | 🟢 | 5 |
| Style Suggestions | 💡 | Multiple |
✍️ Recommended Actions
Before Merge (Blockers)
- ✅ Verify and fix benchmark executable references (
benchmark_librispeech) - ✅ Reconcile WER discrepancies (2.76% vs 1.27%)
- ✅ Create missing referenced files OR remove references:
FLEURS_BENCHMARK.mdPARAKEET_V2_MODEL_CARD.mdREADME_V3_HUGGINGFACE.md
- ✅ Align documentation strategy (Whisper vs Parakeet guides)
After Merge (Follow-up)
- Update
getting-started.mdfor Parakeet - Add dates to benchmark claims
- Consider splitting README into multiple focused docs
- Update CLI help text to match README claims
🎯 Final Verdict
This is excellent work that significantly improves the project's documentation. The content is comprehensive, well-organized, and user-focused. However, there are critical accuracy issues (missing executables, inconsistent WER numbers, broken file references) that must be addressed before merging to avoid user confusion.
Recommendation: Request changes to fix critical issues, then approve.
Great job on this documentation overhaul! Once the critical issues are resolved, this will be a fantastic resource for users. Let me know if you need help creating any of the missing files or reconciling the benchmark numbers. 🚀
|
theres a lot of duplicate information like WER, "fast" english benchmark, and description that you should try to clean up |
PR Review: README UpdateGreat work on transforming the README from work-in-progress to production-ready documentation! This is a significant improvement that makes the project much more accessible. ✅ Strengths1. Excellent Structure & Organization
2. User-Focused Content
3. Comprehensive Technical Details
4. Professional Presentation
|
|
|
🔧 Suggestions for Improvement1. Add System Requirements section Before Quick Start, add explicit requirements:
2. Improve Linux support visibility The documentation feels Windows-first. Consider:
3. Be more specific with performance claims (Line 10) Change "Up to 45× real-time on NPU" to "40-45× real-time on Intel Core Ultra NPU, 5-8× on CPU" to set proper expectations. 4. Add Getting Help section Currently help information is buried. Add a prominent section near the top with Discord, GitHub Issues, and Documentation links. 5. Verify all performance claims before merge Ensure all RTFx numbers, WER percentages, and hardware requirements are tested and reproducible. |
🎯 Action ItemsMust Fix Before Merge:
Should Fix: Nice to Have: 📊 Overall AssessmentQuality: ⭐⭐⭐⭐ (4/5) This is a significant improvement over the WIP README, but needs some technical accuracy fixes before merge. The structure and content are excellent, but the inconsistencies between this README and the project internal guidelines (claude.md) suggest some claims may need verification. Recommendation: Request changes to fix path inconsistencies, platform-specific commands, and verify performance claims before merging. |
README.md
Outdated
| # eddy | ||
|
|
||
| eddy is a C++ inference library designed for native runtimes and multi-vendor edge NPUs, exposing a consistent C++ API plus language bindings for app developers (C#; more to follow). The current milestone focuses on the OpenVINO 2025.x backend for the Parakeet-TDT speech model family while we bring additional runtimes online. | ||
| **eddy** is a high-performance audio-based AI library for Windows and Linux optimized for Intel NPUs and CPUs; eddy powers private, offline automatic speech recognition (ASR) with support for multiple hardware accelerators planned. |
There was a problem hiding this comment.
iirc the encoders for ASR had to be finetuned for GPU or NPU . mutually exclusive.
There was a problem hiding this comment.
becareful of the use of the word “fine tuned” in ML. it’s very overloaded. usually it means you’re doing additional training on the model
PR Review: Update READMESummaryThis PR significantly improves the project documentation by transforming the README from a work-in-progress development guide into a comprehensive, production-ready document. It also adds detailed benchmark results in a separate file. StrengthsExcellent Documentation Quality
Comprehensive Benchmark Data
Critical IssuesInconsistent Benchmark Claims
V3 English WER Discrepancy
Outdated claude.md Reference
High PriorityMissing CPU Benchmark Data
FluidAudio Comparison Needs Clarification
Medium Priority
Security & TestingNo Security Concerns
Test Coverage
Action Items Before Merge
ConclusionStrong documentation update that improves project accessibility and professionalism. Main issues are data consistency problems. Once WER discrepancies are resolved, this will be ready to merge. Recommendation: Request changes to address critical WER inconsistencies, then approve. Overall Assessment: Approve after corrections - Great work, needs data verification. |
README.md
Outdated
| **Usage:** | ||
|
|
||
| ```bash | ||
| # Transcribe with Parakeet V2 |
There was a problem hiding this comment.
i just realized this… btw i’m pretty sure this inspection only works for windows.
to test linux you probably have to run in the “wsl 2” environment.
this also reminds me, I actually forget if intel even releases npu drivers for linux lol. can you double check
There was a problem hiding this comment.
verified, i also added note for we haven't tested linux but it should be supported
There was a problem hiding this comment.
README.md
Outdated
| **Apache 2.0** - See [LICENSE](LICENSE) for details. | ||
|
|
||
| To disable auto-download: set `EDDY_DISABLE_AUTO_FETCH=1` | ||
| Third-party model licenses may vary. See [THIRDPARTY_LICENSES](THIRDPARTY_LICENSES.md) for details on Parakeet TDT models (CC-BY-4.0) and other dependencies. |
There was a problem hiding this comment.
it should be a folder, see fluid audio. also i don’t think you added the file
README.md
Outdated
| ## Links | ||
|
|
||
| Refer to `docs/parakeet_openvino.md` for instructions on pulling the exported model from Hugging Face and running a smoke test through the new `OpenVINOParakeet` wrapper. | ||
| - **Discord**: [Join Community](https://discord.gg/WNsvaCtmDe) |
PR Review: Update README and Add DocumentationSummaryThis PR significantly enhances the project documentation by transforming the README from a work-in-progress technical document into a comprehensive, user-friendly guide. It also adds two new documentation files: BENCHMARK_RESULTS.md and ThirdPartyLicenses/README.md. ✅ Strengths1. Excellent Documentation Quality
2. Strong Benchmark Documentation
3. Proper License Attribution
4. User Experience Focus
🔍 Issues & Concerns1. Data Inconsistencies
|
ThirdPartyLicenses/README.md
Outdated
| - **Description**: C++ package manager for dependency management | ||
|
|
||
| ### CMake | ||
| - **License**: BSD-3-Clause |
There was a problem hiding this comment.
overkill now.
just lost nvidia parakeet whisper and openvino
BENCHMARK_RESULTS.md
Outdated
| Comprehensive benchmark results for eddy ASR on LibriSpeech test-clean and FLEURS multilingual datasets. | ||
|
|
||
| **Hardware**: Intel Core Ultra 7 155H (Meteor Lake) with Intel AI Boost NPU | ||
| **Software**: OpenVINO 2025.x |
Pull Request Review: Update READMEOverviewThis PR significantly improves the documentation and adds valuable supplementary materials. The changes transform the README from a WIP document to a comprehensive, production-ready guide. Overall, this is a strong contribution that enhances the project's usability and professionalism. Positive Highlights1. Documentation Quality ✅
2. Benchmark Results ✅
3. Legal Compliance ✅
4. New Benchmark Script ✅
Issues & Recommendations1. Inconsistency in WER Reporting
|
benchmark_whisper_ov.py
Outdated
| """ | ||
| Benchmark Whisper OpenVINO (eddy optimized) vs PyTorch Whisper | ||
| Compares FluidInference whisper-large-v3-turbo-fp16-ov-npu with OpenAI Whisper | ||
| """ |
benchmark_whisper_ov.py
Outdated
| from pathlib import Path | ||
| import librosa | ||
|
|
||
| def benchmark_whisper_pytorch(audio_files, model_name="base.en"): |
There was a problem hiding this comment.
base.en is pretty useless most of the time. run against the whisper v3 turbo model like the openvino model for a 1-1 comparison?
README.md
Outdated
|
|
||
| ### Benchmarks | ||
|
|
||
| We also provide [Whisper large-v3-turbo OpenVINO](https://huggingface.co/FluidInference/whisper-large-v3-turbo-fp16-ov-npu) optimized for NPU (16× RTFx). Run `python benchmark_whisper_ov.py` to compare Parakeet vs Whisper performance. |
There was a problem hiding this comment.
add a small table for rough RTFx for all 3 models and note the intel chip used in the root README.md so users can quickly get an overview
There was a problem hiding this comment.
also, very surprised that parakeet is only 2x faster?
There was a problem hiding this comment.
which whisper model did you run to get this RTFx
There was a problem hiding this comment.
README.md
Outdated
|
|
||
| See [BENCHMARK_RESULTS.md](BENCHMARK_RESULTS.md) for detailed performance metrics. | ||
|
|
||
| ## Architecture |
There was a problem hiding this comment.
this doesn’t make sense. it doesn’t apply to whisper?
Pull Request Review: Documentation OverhaulOverviewThis PR adds comprehensive documentation including benchmarks, API guides, and third-party licenses. The documentation is well-structured and provides valuable information for users. However, there are several issues that need to be addressed before merging. Critical Issues1. Incorrect Benchmark Executable Reference
|
Pull Request Review: Documentation UpdatesSummaryThis PR adds comprehensive documentation including benchmark guides, results, API documentation, and third-party licenses. Total: 811 lines across 5 new files with significant README updates. Strengths1. Comprehensive Documentation Coverage
2. README Improvements
3. Benchmark Documentation Quality
Critical Issues1. Broken Internal LinkBENCHMARK_RESULTS.md:133 references FLEURS_BENCHMARK.md which does NOT exist. Action Required: Add the missing file or remove the reference. 2. API Documentation Needs VerificationCPP_API.md shows API examples that need verification:
Recommendation: Verify against actual code in include/eddy/ 3. Whisper Support Confusion
Recommendation: Either add Whisper results or move to separate section Major Issues4. Benchmark ReproducibilityPerformance numbers lack reproducibility context:
Recommendation: Add note about expected variance 5. PyTorch Comparison Disclaimer NeededComparison uses estimated numbers:
Recommendation: Add disclaimer or run actual benchmarks 6. Linux NPU Status UnclearSays not tested but provides installation instructions Recommendation: Add clear experimental status warning Minor Issues
RecommendationsBefore Merge (Required)
Nice to Have
Overall AssessmentRating: Good with Required Changes This PR significantly improves documentation. However, accuracy issues must be resolved:
Test Coverage: N/A (docs only) Files Reviewed:
Reviewed by: Claude Code Assistant |
Pull Request Review: Documentation UpdateThis PR significantly improves the project documentation by adding comprehensive guides and expanding the README. Overall, this is a valuable contribution that will help users get started and understand the project better. ✅ Strengths
🔍 Issues & SuggestionsBENCHMARK.mdLine 89: Typo in RTFx explanation Should be: "10 seconds" not "6 seconds" (60 seconds / 10 RTFx = 6 seconds is correct, but the wording implies 1 min becomes 6 seconds) Line 197-198: Inconsistent command format # Manual download with error handling
wget https://www.openslr.org/resources/12/test-clean.tar.gz || echo "Download failed"
tar -xzf test-clean.tar.gzBENCHMARK_RESULTS.mdLine 27: Consider adding confidence intervals Line 47: Missing citation year Add reference to when/where this benchmark was published. Lines 62-84: Language table formatting README.mdLine 99: Footnote formatting inconsistency Lines 101-102: Unverified claims Using "proxy" and "estimated" weakens the comparison. Either provide actual benchmarks or clearly mark this section as "estimated" in the table header. Line 131: Untested claim This is honest but should include a call-to-action for community testing. Line 164: Year inconsistency year={2025}We're in 2025, but the project may have started earlier. Verify the correct year. CPP_API.mdLine 21: Missing error handling in example try {
auto asr = eddy::ParakeetASR::create("parakeet-v2", "NPU");
auto result = asr->transcribe("audio.wav");
std::cout << "Text: " << result.text << std::endl;
} catch (const std::exception& e) {
std::cerr << "Error: " << e.what() << std::endl;
}Line 247: Broken links ThirdPartyLicenses/README.mdLine 25: Missing license details 🐛 Potential Issues
🔒 Security ConcernsNo security issues identified. The documentation properly attributes third-party licenses and doesn't expose sensitive information. ⚡ Performance ConsiderationsThe documentation accurately represents performance characteristics. However:
✅ Test CoverageThis PR is documentation-only, so no test coverage changes. However, consider:
📋 Recommendations
🎯 Overall AssessmentQuality: 8/10 - Excellent documentation effort with comprehensive coverage This PR significantly improves the project's documentation and makes it much more accessible to users. With the suggested fixes, this will be an excellent addition. Great work! Recommendation: ✅ Approve with minor revisions cc: @Alex-Wengg |
No description provided.