Skip to content

Support hf generate #12477

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

Merged
merged 8 commits into from
Dec 4, 2024
Merged

Support hf generate #12477

merged 8 commits into from
Dec 4, 2024

Conversation

hkvision
Copy link
Contributor

@hkvision hkvision commented Dec 2, 2024

original generate -> simple_generate
current generate -> hf generate

@jason-dai
Copy link
Contributor

Please also test other models.

@hkvision
Copy link
Contributor Author

hkvision commented Dec 2, 2024

Please also test other models.

Sure.

@hkvision
Copy link
Contributor Author

hkvision commented Dec 4, 2024

Current usage:

  • Default to use simple=True, if want to benchmark, need to add do_print=True
  • If change to simple=False, will change to hf generate. In this way, can't be used together with BenchmarkWrapper, because BenchmarkWrapper's generate doesn't have the simple argument, will have the following error:
File "C:\Users\arda\miniforge3\envs\kai-acc-lib\lib\site-packages\ipex_llm\utils\benchmark_util_4_29.py", line 1180, in _validate_model_kwargs
raise ValueError(
ValueError: The following model_kwargs are not used by the model: ['simple'] (note: typos in the generate arguments will also show up in this list)
  • If want to benchmark hf generate performance, just add BenchmarkWrapper without specifying simple=True when generate.

@jason-dai

@jason-dai
Copy link
Contributor

  • If change to simple=False, will change to hf generate. In this way, can't be used together with BenchmarkWrapper, because BenchmarkWrapper's generate doesn't have the simple argument, will have the following error:

Can we make simple a kwarg instead?

@hkvision
Copy link
Contributor Author

hkvision commented Dec 4, 2024

Won't impact current all-in-one, in all-in-one, will default always to test simple generate.
If want to test hf generate, may need to manually remove the if condition here: https://github.com/intel-analytics/ipex-llm/blob/main/python/llm/dev/benchmark/all-in-one/run.py#L650C5-L651C40 or add a new option in config.yaml.
Pending to support this if needed in the future.

@hkvision
Copy link
Contributor Author

hkvision commented Dec 4, 2024

  • If change to simple=False, will change to hf generate. In this way, can't be used together with BenchmarkWrapper, because BenchmarkWrapper's generate doesn't have the simple argument, will have the following error:

Can we make simple a kwarg instead?

Good idea, will modify this.

@hkvision
Copy link
Contributor Author

hkvision commented Dec 4, 2024

  • If change to simple=False, will change to hf generate. In this way, can't be used together with BenchmarkWrapper, because BenchmarkWrapper's generate doesn't have the simple argument, will have the following error:

Can we make simple a kwarg instead?

Updated.

Still simple can't work together with BenchmarkWrapper, the code looks better after making it a kwarg.

Copy link
Contributor

@jason-dai jason-dai left a comment

Choose a reason for hiding this comment

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

LGTM

@hkvision
Copy link
Contributor Author

hkvision commented Dec 4, 2024

Merge it first. Will fix llama3.2 issue in a next PR very soon.

@hkvision hkvision merged commit 7ff4533 into intel:main Dec 4, 2024
1 check passed
@hkvision hkvision deleted the generate branch December 4, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants