Skip to content

Add custom sampler order support#5443

Merged
oobabooga merged 17 commits into
devfrom
sampler_order
Feb 6, 2024
Merged

Add custom sampler order support#5443
oobabooga merged 17 commits into
devfrom
sampler_order

Conversation

@oobabooga
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

kalomaze commented Feb 7, 2024

Thank you for adding this🙏

@biship
Copy link
Copy Markdown

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