docs: 添加了miloco推理管线onnx需要本地推理的说明 - #388
Conversation
|
👋 感谢提交 PR @Kylehub-Kyle!维护者会尽快 review。 提交前请确认:
|
PR #388: docs: 添加了 miloco 推理管线 onnx 需要本地推理的说明作者: Kylehub-Kyle 修改方案纯文档改动(
commit 线(5 条): 复核记录(本轮)
问题本轮无 🔴 / 🟡 / 🔵 问题——上轮唯一的 🔵(英文语法病句)已在本轮 diff 修复。 结论LGTM — 纯文档澄清。核心技术声明经 origin/main 知识库复核属实(检测 / ReID 为必需的本地 ONNX 模型),两段无冲突,PR body 与 diff 一致,5 条 commit message 对齐;上轮 ci 提的英文语法问题( 由 review-pr skill v1.6 生成 |
|
@Kylehub-Kyle 感谢提交PR,请关注下ci review 提的不一致问题。另外更新了具体的设备,有具体的实测么? |
There was a problem hiding this comment.
Thanks for the PR! Spelling out the CPU / local-inference requirements is genuinely helpful — "why do I need a decent CPU if it's cloud-based?" trips up a lot of first-time deployers. Good direction 👍
One thing I'd like to align on before merging, plus two suggestions:
1. [Needs fix] The three docs disagree on the x86 minimum
sdk-onnxruntime.mdtable: x86 Linux/Windows → minimum Celeron N5105 / recommended i3 10th-gen+- but
dev-guide.mdsaysx86 i3 8th-gen+, andREADME(EN + ZH) says "x86 needs i3 8th-gen or newer"
These don't line up, so a reader can't tell whether an N5105 actually qualifies (the table says yes, README/dev-guide imply no). It looks like the x86 macOS row's i3-8th-gen got treated as the bar for all x86, dropping the x86 Linux/Windows row. Suggest making the table the single source of truth and having the README/dev-guide summaries either say "see table" or distinguish the two x86 platforms the same way the table does.
2. [Question] Are the x86 tiers measured or estimated?
I saw from the group chat that you've tested the Raspberry Pi tiers, so this is just about the x86 rows. The only value flagged as "empirically calibrated" in the diff is the ONNX thread count; the x86 CPU tiers (N5105 minimum, i3-10th-gen+ recommended, Atom/Z8350 ">200ms") don't cite a basis. If they're measured, could you note the test hardware? If they're estimates, I'd suggest softening "minimum/recommended" to "rough guidance" and noting they haven't been validated across all chips — so nobody buys an N5105 as the "minimum" and finds it can't keep up.
3. [Suggestion] Ordering of the CAUTION block
The original CAUTION contained only the cost note; this version puts the new "local ONNX inference required" paragraph first and pushes the cost note down to second. Order affects what the reader's eye lands on first. If there wasn't a specific reason, I'd suggest appending the new ONNX paragraph after the cost note (keeping cost first); if you do think local inference deserves top billing, that's fine too — just call out the trade-off in the description. Also, "not purely cloud-based" sitting right next to "cloud-based models … incur costs" reads a little at odds; might be worth a light polish.
(The "Hardware" bullet inserts CPU right after storage and keeps the existing order intact — that part's fine.)
Thanks again 🙏
The existing caution only mentioned cloud API costs, making it easy to assume all inference is cloud-based. Add a note that the perception pipeline runs person detection and re-identification models locally via ONNX Runtime.
668bbcf to
b7e680a
Compare
|
很感谢大佬能抽空审查,这是我第一次使用GitHub,仍有许多操作不明白,依赖群内机器人操作,很担心会因此给研发组带来额外的工作负担。真心感谢大佬的审核,真的学到很多。 |
|
@Kylehub-Kyle 感谢你的PR,caution block 是专门用来提醒用户注意的事项,这里是注意成本,onnx 本地推理不应该放到 caution block里面,不需要专门提醒,但可以写到其他地方,做个说明是可以的。 |
Uh oh!
There was an error while loading. Please reload this page.