feat: Support mrope_section with rope_type: "yarn"#13313
feat: Support mrope_section with rope_type: "yarn"#13313ispobock merged 13 commits intosgl-project:mainfrom
mrope_section with rope_type: "yarn"#13313Conversation
49312c4 to
3c28a31
Compare
3c28a31 to
7dc040c
Compare
|
Test locally with the same setup as #13238, on basic testing everything seems to be OK and functional now. |
|
When it comes to testing, we can make changes to |
@raayandhar Your running command have no problem. The test_mrope was broken due to some recent PR merged. We are trying to fix it. Currently have no workaround. Sorry for making confusion. |
No worries, thanks so much for the clarification. Once there's a fix I'll add a test. I was able to get the test passing with the changes I mentioned above, if that helps. But that not be the correct way to do it. |
Is there some timeline for the fix? I am also happy to try and help. |
1e3c382 to
5e41c35
Compare
|
This PR can be re-reviewed once we merge #14082 and I add yarn scaling to the unit test. |
5e41c35 to
7cbf330
Compare
|
Now that we have merged the fix, I adapted the UT test results (click to expand)I think this PR is ready for review / CI now @yhyang201 cc @yuan-luo |
|
Please paste the lmms_eval result. |
|
/tag-and-rerun-ci |
Running introduced We are passing with score of 0.4644, but you can expand below: Test results (click to expand) |
|
Caught a small extra print, but before that nearly all the CI was passing, only one that failed was not related to my changes. |
|
@yhyang201 could you take another look? |
|
/rerun-failed-ci |
|
@yhyang201 could you take a look at the CI and let me know what you think? The failures seem unrelated. |
|
Could you fix the VLM CI? Thanks. |
Signed-off-by: Raayan Dhar raayan.dhar@gmail.com <raayan.dhar@gmail.com>
I don't think the CI is related to my changes. I just tested on main and got the same failure (0.3833 below 0.4): Test results (click to expand)Also, the model config does not use YaRN scaling, so it should not hit my path anyways. The root cause of this CI failing should be separate from my changes. |
|
/rerun-failed-ci |
|
Could you paste a model leveraging this flag and the acc gets no drop? This is a critical PR, we are very cautious to merge. Thank you very much. |
Sure, I can do so later today. Actually earlier I also pasted the lmm_evals result as well for a model using this flag. Is that sufficient or were you thinking of something else? |
|
/rerun-failed-ci |
|
Let's have all the GPU CI passed. |
|
Overall, LGTM. |
|
/rerun-failed-ci |
1 similar comment
|
/rerun-failed-ci |
|
/rerun-failed-ci |
2 similar comments
|
/rerun-failed-ci |
|
/rerun-failed-ci |
|
/rerun-failed-ci |
…3313) Signed-off-by: Raayan Dhar raayan.dhar@gmail.com <raayan.dhar@gmail.com> Signed-off-by: raayandhar <raayan.dhar@gmail.com>
…3313) Signed-off-by: Raayan Dhar raayan.dhar@gmail.com <raayan.dhar@gmail.com> Signed-off-by: raayandhar <raayan.dhar@gmail.com>
Motivation
Support for this feature, as described: #13219
Modifications
New class:
YaRNScalingMRotaryEmbeddingto do MRoPE-enabled rotary embedding with YaRN context scaling. If we see amrope_sectionandscaling_type == "yarn"then we can use this new class, otherwise we default back toYaRNScalingRotaryEmbedding.Accuracy Tests
Added unit test
Benchmarking and Profiling
Checklist