akaashrp commented on PR #19771: URL: https://github.com/apache/tvm/pull/19771#issuecomment-4953047944
Thanks for iterating on this. I think the PR is doing too many unrelated things and should be narrowed to chunked tensor-cache loading. The pieces that seem directly relevant are: 1. Planning chunks based on both encoded and decoded sizes. 2. Keeping WebGPU offsets and lengths aligned. 3. Cleaning up chunk views correctly. 4. Respecting `cpu_arr->byte_offset` in `ArrayDecodeStorage`, since chunks are decoded into views with nonzero offsets. I don’t think we should also change the meaning of `f32-to-bf16` here. Accepting a native four-byte float32 payload under that format makes the interface ambiguous, and it would behave differently from the C++ and Python loaders. The original [Gemma checkpoint](https://huggingface.co/google/gemma-4-E2B/blob/main/model.safetensors) is entirely BF16. From the reported names, shapes, and sizes, I would expect the converted tensors to use the usual q4f16 representation: packed 32-bit `q_weight` values and 16-bit scales. Could you share the relevant `tensor-cache.json` entries and identify the record that actually needs the new float32 fallthrough? I would expect it to have metadata like: ``` dtype = float32 format = f32-to-bf16 nbytes = product(shape) × 4 ``` If such a record exists, why can't it be tagged as `raw`? Either way, this seems like a separate format-compatibility change that should have its own discussion. The Shape callback and general int64 marshalling changes also don't appear necessary for chunking and should be split out unless there is a direct dependency I'm missing. I'm also not yet convinced we know where the original failure occurred. Seeing `arrayDecodeStorage:start` without `arrayDecodeStorage:done` only tells us that the overall packed call did not return. The failure could be while growing the JS call stack, allocating Wasm argument space, copying into Wasm memory, entering C++, or performing the actual decode. The current path does create several record-sized temporary allocations, so memory pressure is a reasonable theory, but we should identify the operation that actually fails. There is also an issue with the reported 1120 MiB tensor. TVM requests only 1 GiB for `maxBufferSize` and `maxStorageBufferBindingSize` in `web/src/webgpu.ts`. Chunking does not split the final GPU tensor and still allocates a full `gpu_arr`. Could you log both the adapter limits and the actual device limits? If the successful test used a sharded version of this tensor rather than one `[262144, 1120]` allocation, please clarify that and update the PR description. Could you provide a reproducible comparison showing, with the same artifact and environment: 1. The current full-record path fails at a specific, identified operation. 2. PR head with chunking disabled fails in the same way. 3. PR head with chunking enabled succeeds. 4. The final GPU allocation fits the actual device limits and is successfully used. 5. For a smaller record, such as 256 MiB, full and chunked loading produce identical tensor contents. Finally, 128 MiB still looks arbitrary. Could you try a few chunk sizes (perhaps 32, 64, 256, 512, 1024 MiB) and report which ones work and where failures begin (if at all)? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
