Support Qwen3 (str.startswith() and [::-1])#66
Conversation
taha-yassine
changed the title
|
I added support for step=-1 in slice since ggml-org/llama.cpp#13181 was closed. |
ochafik
left a comment
ochafik
left a comment
There was a problem hiding this comment.
This is awesome, thanks @taha-yassine!!!
include/minja/minja.hpp
Outdated
| if (auto slice = dynamic_cast<SliceExpr*>(index.get())) { | ||
| auto start = slice->start ? slice->start->evaluate(context).get<int64_t>() : 0; | ||
| auto end = slice->end ? slice->end->evaluate(context).get<int64_t>() : (int64_t) target_value.size(); | ||
| bool reverse = slice->step && slice->step->evaluate(context).get<int64_t>() == -1; |
There was a problem hiding this comment.
nit: for readability I'd maybe first extract step (default to 1), then do the reverse + explosion if step not in (1,-1)
include/minja/minja.hpp
Outdated
| int64_t start = slice->start ? slice->start->evaluate(context).get<int64_t>() : (reverse ? target_value.size() - 1 : 0); | ||
| int64_t end = slice->end ? slice->end->evaluate(context).get<int64_t>() : (reverse ? -1 : target_value.size()); | ||
|
|
||
| size_t len = target_value.size(); |
There was a problem hiding this comment.
move this one up to use it instead of target_value.size() calls
|
@taha-yassine I've taken the liberty to simplify the code a bit (now supports any step != 0) + test strings, |
Awesome thanks! Related to the out-of-bounds issue I mentioned, it seems that your code doesn't handle that case? I wrote a little test to verify and it doesn't pass. EXPECT_EQ(
"[0, 1, 2, 3][0, 1, 2, 3][]",
render("{% set x = [0, 1, 2, 3] %}{{ x[-10:] }}{{ x[:10] }}{{ x[10:20] }}", {}, {}));Or maybe the complexity it adds isn't worth it? |
* minja: sync google/minja@f06140f - google/minja#67 (@grf53) - google/minja#66 (@taha-yassine) - google/minja#63 (@grf53) - google/minja#58 --------- Co-authored-by: ochafik <ochafik@google.com>
* minja: sync google/minja@f06140f - google/minja#67 (@grf53) - google/minja#66 (@taha-yassine) - google/minja#63 (@grf53) - google/minja#58 --------- Co-authored-by: ochafik <ochafik@google.com>
* Add str.startswith() * Add support for step=-1 in slice * Add Qwen3 template * Clamp out-of-bounds slice indices * Simplify subscript logic + handle any (non-zero) step --------- Co-authored-by: Olivier Chafik <ochafik@users.noreply.github.com>
2 participants
This PR:
.startswith()Closes #64
Addresses ggml-org/llama.cpp#13178