Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The unresolved KV-prefix transfer issue and missing required dtype conversion in the Ascend quickstart should be addressed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds Ascend CANN support for SenseNova U1.5 with configurable backends, VRAM limits, dtype conversion, and deployment tooling.
Changes:
- Adds backend and per-device memory configuration.
- Adds KV-prefix staging and FP16/FP32 generation conversion.
- Adds CANN container support, dependency updates, and documentation.
File summaries
| File | Summary |
|---|---|
src/session.cpp |
Configures backend, VRAM, and logging options. |
src/llama-cpp-adapter.cpp |
Adds backend selection and KV-prefix staging. |
src/cli.cpp |
Exposes new runtime options. |
scripts/convert-model.py |
Adds generation dtype conversion. |
README.md |
Documents CANN setup, build, and execution. |
include/session.h |
Extends the session configuration API. |
include/llama-cpp-adapter.h |
Updates adapter and prefix-storage types. |
docs/research/sensenova-u15-ascend-debug-status.md |
Records Ascend validation results. |
docs/research/sensenova-u1-8b-mot-porting-notes.md |
Documents model-porting constraints. |
docker/requirements-cann-mvp.txt |
Defines CANN dependencies. |
docker/Dockerfile.cann-mvp |
Defines the CANN 8.5.0 image. |
dependencies.json |
Pins Ascend engine revisions. |
.gitmodules |
Updates engine branches. |
.gitignore |
Ignores generated outputs and caches. |
Review details
Suppressed comments (2)
README.md:109
- This sequence builds the Docker image, but the following
cmakecommands run directly on the caller's host. The Dockerfile only installs dependencies and sets a workdir, so a host without the CANN SDK cannot compile from these instructions and the supplied image is never used. Add adocker run/docker execbuild step with the checkout mounted, or explicitly document the required host-side CANN installation.
cmake -S . -B build-cann -DCMAKE_BUILD_TYPE=Release \
-DGGML_CANN=ON -DSOC_TYPE=Ascend310P3
cmake --build build-cann -j 8
src/llama-cpp-adapter.cpp:392
- This unconditionally calls
ggml_backend_tensor_getfor every key and value, adding a full device-to-host transfer for every prefix export.transfer_prefixperforms this for each conditioning slot (and up to three slots for editing), including the existing CUDA/single-device path where the previous code kept the KV tensors backend-resident. Keep a backend-compatible fast path and only stage through host memory when the selected engines cannot share device tensors.
auto stage_to_host = [&](ggml_tensor * source) {
auto * host = ggml_dup_tensor(result.descriptors.get(), source);
result.host_storage.emplace_back(ggml_nbytes(source));
auto & bytes = result.host_storage.back();
ggml_backend_tensor_get(source, bytes.data(), 0, bytes.size());
- Files reviewed: 15/16 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+145
to
+147
| - Generation weights retain their source dtype and values. Use | ||
| `--generation-outtype f16` or `--generation-outtype f32` to convert BF16 | ||
| generation weights when required by the target backend. |
coxine
force-pushed
the
ascend-310p-u15-npu
branch
from
September 17, 2026 08:09
8d7ccf7 to
f86398a
Compare
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.
Assisted-by: OpenAI Codex
Adds configurable generation backends and per-device VRAM limits, CANN 8.5.0 container support, FP32 generation conversion, and Ascend deployment documentation.
Validated for text and image generation on two Ascend 310P devices. The 2048×2048, 50-step image test completed entirely on NPU and produced a correct output.