-
Notifications
You must be signed in to change notification settings - Fork 2.1k
enable method comparison to device-agnostic, to enable it on other accelerators like xpu #2606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: YAO Matrix <matrix.yao@intel.com>
BenjaminBossan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I'm unsure at the moment if we want to make the PEFT method comparison code device-agnostic. Our plan is to always run it on the same device to make it possible to compare the results in a fair manner. Even if users submit their own experiments, we would still run it ourselves. As CUDA is still the dominant platform, there is thus little need for the code to be device agnostic.
That said, I can see that the changes are quite small and there are some advantages for users who want to run this at home who don't have NVIDIA GPUs. @githubnemo WDYT?
|
I'm in favor of supporting XPU. Regarding official benchmark numbers, if there was a device that we could run the benchmarks on, I would even go as far as maintaining a second set of benchmarks. But there doesn't seem to be a way, currently, so this feature will only benefit those people who want to run the benchmarks locally. |
--------- Signed-off-by: YAO Matrix <matrix.yao@intel.com>
FIX Transformers VLM architecture changes Follow up to huggingface#2554 See discussion in huggingface/transformers#38627 To quote: > transformers PR #37033 re-arranges the way visual language models are built by moving the LM head from the language model to the top-level VLM (among other things). A consequence of this is that the keys in the PEFT state_dict now also follow the new architecture. This means that: 1. If a PEFT checkpoint was saved with the old architecture but is loaded with the new architecture, loading fails. 2. If a PEFT checkpoint was saved with the new architecture but is loaded with the old architecture, loading fails. 1. can be addressed by making use of the newly added _checkpoint_conversion_mapping attribute for models with the new architecture. In transformers, this is used to map old model state_dicts to the new state_dict format. In PEFT, with some fiddling, we can use the same mapping to make old PEFT state_dicts compatible with the new architecture (backwards compatibility). However, 2. is not easily addressed. We would need a reverse mapping for this. This could be easily derived from _checkpoint_conversion_mapping, but since this attribute doesn't exist on old models, we cannot do that. Therefore, new checkpoints created with PEFT on these models won't load successfully when users use old transformers (forward compatibility). These cases are covered by the added unit tests, which means that the test covering case 2 are marked as xfail. If we could reliably detect that we are in case 2, we could warn the user and advise them to upgrade transformers, but I don't know if it's possible to figure this out. We also allow users to pass their own key_mapping to from_pretrained and load_adapter, though the documentation advises against it. This argument could theoretically be used as a workaround in case there is indeed an issue with prompt learning state_dicts. Apart from these changes, I also made a small change to account for huggingface/transformers#38017 (comment).
--------- Signed-off-by: jiqing-feng <jiqing.feng@intel.com>
This change updates all results with their respective number of parameters (trained + absolute) and adds the newly introduced full-finetuning. In addition to these results there was also an issue with the Makefile as it didn't consider the possibility of having experiments that don't have an adapter config (e.g., full fine-tuning).
Signed-off-by: YAO Matrix <matrix.yao@intel.com>
* initial commit * doc on custom reward function * test * doc doc doc * fix collator * style * links? * I need a docdoc 🎵 * fix link * I do like writing doc tbh * it takes time, but it's worth it * no return! * type hint * it's probably the best of both worlds [ci skip] * new doc before implementation * tests * more doc * style * multiple pretrained funcs * fix arg name * main? * example for R1 * fix script * clearer * import [ci skip] * Update docs/source/grpo_trainer.md Co-authored-by: lewtun <lewis.c.tunstall@gmail.com> --------- Co-authored-by: lewtun <lewis.c.tunstall@gmail.com>
@BenjaminBossan , pls help review, thx very much.