Skip to content

Conversation

@f291400
Copy link
Contributor

@f291400 f291400 commented Dec 30, 2025

Make sure to read the contributing guidelines before submitting a PR

Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also wait for @CISC @ggerganov reviews for libllama changes

@f291400 f291400 requested review from CISC and ngxson December 31, 2025 01:16
@github-actions github-actions bot added the model Model specific label Dec 31, 2025
Copy link
Collaborator

@CISC CISC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after whitespace fixes

@f291400
Copy link
Contributor Author

f291400 commented Dec 31, 2025

So should I revert to the previous state, or leave it as it is?

@f291400 f291400 closed this Dec 31, 2025
@f291400 f291400 reopened this Dec 31, 2025
@CISC
Copy link
Collaborator

CISC commented Dec 31, 2025

So should I revert to the previous state, or leave it as it is?

Revert, but don't touch ggml/src/ggml-sycl/CMakeLists.txt.

@CISC
Copy link
Collaborator

CISC commented Dec 31, 2025

Perfect.

@caohaoyuabcd

This comment was marked as off-topic.

@CISC

This comment was marked as off-topic.

@f291400
Copy link
Contributor Author

f291400 commented Dec 31, 2025

I'd like to inquire when this model will be supported. I see that it has been open-sourced, and I want to try it out as soon as possible. https://huggingface.co/tencent/Youtu-LLM-2B

When it is ready and merged, however it also requires support in minja for its chat template: ochafik/minja#24

Thanks. I will change rsplit to split in chat_template.json.

@CISC
Copy link
Collaborator

CISC commented Dec 31, 2025

Thanks. I will change rsplit to split in chat_template.json.

Don't, just use it properly instead, as I suggested here: https://huggingface.co/tencent/Youtu-LLM-2B/discussions/1

@f291400
Copy link
Contributor Author

f291400 commented Dec 31, 2025

@ngxson @ggerganov
Could you please review the code and check if there are any remaining bugs that need to be addressed? I would like to merge these changes into the main branch as soon as possible.

@ngxson
Copy link
Collaborator

ngxson commented Jan 1, 2026

Lint CI fails, please fix before we can merge.

@CISC
Copy link
Collaborator

CISC commented Jan 1, 2026

Lint CI fails, please fix before we can merge.

LOL, I think it just picks up the previous master error somehow...

@f291400 Try rebasing, should fix the CI.

@ngxson
Copy link
Collaborator

ngxson commented Jan 1, 2026

If it's fixed on master then I think it's ok to merge as-is then @CISC ?

Beside, @f291400 if you want reviews to be fast and efficient, read the contribution guidelines and validate your changes carefully.

This PR is create from your master branch, maintainers cannot push fixes directly here; the PR is also moved 2-3 times which make our work extremely inefficient.

@CISC
Copy link
Collaborator

CISC commented Jan 1, 2026

If it's fixed on master then I think it's ok to merge as-is then @CISC ?

I ran flake8 locally and it doesn't report any errors, and the line number given by CI is bogus, so I think it should be fine.

@svlys
Copy link

svlys commented Jan 1, 2026

Thank you for acknowledging this submission, which will greatly advance the use of youtu-llm on llama.cpp.

@ngxson
Copy link
Collaborator

ngxson commented Jan 1, 2026

I attempted to merge via GH web UI but failed, so unfortunately you need to fix the merge conflict yourself @f291400

@CISC
Copy link
Collaborator

CISC commented Jan 1, 2026

This PR is create from your master branch, maintainers cannot push fixes directly here; the PR is also moved 2-3 times which make our work extremely inefficient.

So, it seems GitHub started allowing this now?

@ngxson
Copy link
Collaborator

ngxson commented Jan 1, 2026

So, it seems GitHub started allowing this now?

No idea, probably allowed via web UI only?

I never have problem applying patches via web UI. But if I do gh pr checkout locally and if the PR is created from fork's master branch, I always get permission error on git push

@CISC
Copy link
Collaborator

CISC commented Jan 1, 2026

So, it seems GitHub started allowing this now?

No idea, probably allowed via web UI only?

I never have problem applying patches via web UI. But if I do gh pr checkout locally and if the PR is created from fork's master branch, I always get permission error on git push

I'm pretty sure merging from master used to fail.

@ngxson ngxson merged commit ced765b into ggml-org:master Jan 1, 2026
72 checks passed
srogmann pushed a commit to srogmann/llama.cpp that referenced this pull request Jan 1, 2026
* Support Youtu-VL Model

* merge code

* fix bug

* revert qwen2 code & support rsplit in minja.hpp

* update warm info

* fix annotation

* u

* revert minja.hpp

* fix

* Do not write routed_scaling_factor to gguf when routed_scaling_factor is None

* fix expert_weights_scale

* LGTM after whitespace fixes

* fix

* fix

* fix

* layers to layer_index

* enum fix

---------

Co-authored-by: Xuan-Son Nguyen <[email protected]>
Co-authored-by: Sigbjørn Skjæret <[email protected]>
ronaldmannak pushed a commit to PicoMLX/llama.cpp that referenced this pull request Jan 2, 2026
* Support Youtu-VL Model

* merge code

* fix bug

* revert qwen2 code & support rsplit in minja.hpp

* update warm info

* fix annotation

* u

* revert minja.hpp

* fix

* Do not write routed_scaling_factor to gguf when routed_scaling_factor is None

* fix expert_weights_scale

* LGTM after whitespace fixes

* fix

* fix

* fix

* layers to layer_index

* enum fix

---------

Co-authored-by: Xuan-Son Nguyen <[email protected]>
Co-authored-by: Sigbjørn Skjæret <[email protected]>
ronaldmannak pushed a commit to PicoMLX/llama.cpp that referenced this pull request Jan 9, 2026
* Support Youtu-VL Model

* merge code

* fix bug

* revert qwen2 code & support rsplit in minja.hpp

* update warm info

* fix annotation

* u

* revert minja.hpp

* fix

* Do not write routed_scaling_factor to gguf when routed_scaling_factor is None

* fix expert_weights_scale

* LGTM after whitespace fixes

* fix

* fix

* fix

* layers to layer_index

* enum fix

---------

Co-authored-by: Xuan-Son Nguyen <[email protected]>
Co-authored-by: Sigbjørn Skjæret <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples model Model specific python python script changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants