Skip to content
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

Add custom sampler order support #5443

Merged
merged 17 commits into from
Feb 6, 2024
Merged

Add custom sampler order support #5443

merged 17 commits into from
Feb 6, 2024

Conversation

oobabooga
Copy link
Owner

@oobabooga oobabooga commented Feb 5, 2024

What this PR implements:

  • Sorting samplers according to a custom priority list, provided as a string. The values must be separated by new lines or commas. Example:
temperature
top_k
top_p
typical_p
epsilon_cutoff
eta_cutoff
tfs
top_a
min_p
dynamic_temperature
quadratic_sampling
mirostat
  • Make it possible to use temperature, dynamic temperature, and quadratic sampling at the same time.
  • Make it possible to use mirostat with other samplers (previously, all parameters except for temperature were discarded).
  • Deprecate temperature_last, since it's no longer necessary. (I'll keep it for now) For now, it moves temperature, dynamic_temperature, and quadratic_sampling to the end of the list, in an arbitrary order.
  • Make the custom sampler order part of presets.

What this PR does not implement:

  • Changing the order of logits processors like repetition penalty and frequence penalty. They are always applied before the logits warpers above.
  • Adding more than 1 copy of a warper to the stack.
  • Adding more than 1 copy of a warper to the stack where each copy is initialized with different parameters.

@oobabooga oobabooga mentioned this pull request Feb 5, 2024
@BadisG
Copy link
Contributor

BadisG commented Feb 5, 2024

First of all, thanks a lot for deciding to go for that feature #5403 (comment), much appreciated.

I did some tests with it and here's my suggestions:

1. "dynamic_temperature" is still there

Yeah, like you said, that one needs to be removed in the UI, it's still there for the moment (for the _hf loaders).

2. This PR broke the non "_hf" loaders

The samplers aren't working anymore, I was trying temperature = 5 on llamacpp and it still gave me the same output as temperature = 1
Edit: I'm on the dev branch now and it seems like llamacpp is just broken regardless or anything...

3. The list should be dynamic for the different loaders

llamacpp doesn't have quadratic sampling for example so it should not appear on the sampler order list

4. Suggestions for some UI improvement

You made that feature in a few hours so of course I'm aware that it's for the moment a draft that we can work on. Once you've spent a bit more time in the UI, here's how I envison the look and usage with it (I'll show that with my flawless photoshop skills :^)
Test_sampler_order_UI

  1. The samplers could be in some sort of rectangles so that we can change the order of a specific one by clicking on it (the border becomes green) + clicking on an up/down arrow, or by simply dragging that sampler box upwards or downards with the cursor.
  2. The samplers that are in "Off" mode (like temp = 1 -> doesn't interfere with the logits) shouldn't appear on the sampler order UI, that would make the experience more simple since we are usually working with 4-5 max samplers at the same time.

@oobabooga
Copy link
Owner Author

This PR doesn't touch non-HF loaders. About UI, I agree that it could be improved, but gradio doesn't have an easy component for drag and drop a stack of values. That can be added later.

You don't have to provide all parameters every time. If you are only using min_p, top_p, and temperature, you can write

top_p
temperature
min_p

@kalomaze
Copy link
Contributor

kalomaze commented Feb 7, 2024

Thank you for adding this🙏

@biship
Copy link

biship commented Feb 7, 2024

IMO having a text box list of samplers works fine.
Perhaps add text about positioning of rep_pen & freq_pen?
Worth logging when a sampler doesn't exist? (to catch when people make typos).
Any need to update the preset yamls with their sampler ordering?

This is great change, thanks for it.

@oobabooga oobabooga deleted the sampler_order branch February 9, 2024 05:07
PoetOnTheRun pushed a commit to PoetOnTheRun/text-generation-webui that referenced this pull request Feb 22, 2024
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.

4 participants