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

server : (web UI) Add back sampler settings #10239

Merged
merged 4 commits into from
Nov 10, 2024

Conversation

MaggotHATE
Copy link
Contributor

This PR adds samplers and penalties back into new server UI - I'm taking small iterative steps with this since I'm not familiar with vue yet. I've divided them into two "sections" for better distinction.

@slaren
Copy link
Member

slaren commented Nov 9, 2024

It's good to add more sampler options, but IMO this UI should focus strictly on usability. As it is, a list of options without any explanation is not much better than the "custom JSON config" field.

@MaggotHATE
Copy link
Contributor Author

not much better than the "custom JSON config" field

It's already better by not having to print or copy-paste "xtc_probability" manually XD
In general, this simplifies organizing payload from this UI, considering that it is the main server UI now - and it's hidden enough to not distract when not needed.

As it is, a list of options without any explanation

If I were to add "tooltips" for samplers, should I also consider mobile use (i.e. vertical orientation/aspect ratios)?

@MaggotHATE
Copy link
Contributor Author

MaggotHATE commented Nov 10, 2024

@slaren @ngxson
I've added simple tooltip buttons to parameters; the information itself can be adjusted easily later, but this way it should be accessible both from mouse and touch input. Will also look into fixing stretching input fields back. Should be fixed.

image

@ngxson ngxson changed the title server: Add back samplers server : (web UI) Add back sampler settings Nov 10, 2024
@MaggotHATE
Copy link
Contributor Author

Thank you for fixing the name, forgot to change it.

The only thing that bugs me here is the scrollbar. It appears that there's no universal way to change it (and it's not a built-in element, like in ImgUi). Is it possible to shift settings content in order to always reserve a space for the scrollbar?

@ngxson
Copy link
Collaborator

ngxson commented Nov 10, 2024

I pushed some changes:

  • Input field is not wrapped inside a component (prevent too many repetitions in the code)
  • Help message now show on hover, so no modal is needed:
    image
  • Re-group settings to top-level collapse sections:
    image

Can you test it once more time to confirm that it's still working correctly @MaggotHATE ? Then I think it's good to merge. Thank you.

@MaggotHATE
Copy link
Contributor Author

Can you test it once more time to confirm that it's still working correctly

@ngxson looks like everything works correctly, thank you!

Would tooltips work on mobile though? So far this UI is compatible with touch, even though it's not the main focus for now. It was the only reason I went with modals.

@ngxson
Copy link
Collaborator

ngxson commented Nov 10, 2024

I haven't tested, but because the tooltip is implemented using dropdown component from daisyui, it should be toggle it by touch too.

tailwindcss make it quite easy to make responsive UI though, so normally I don't have to worry too much about that. We can fix responsiveness in the future if users really need it.

@ngxson ngxson merged commit 505f332 into ggml-org:master Nov 10, 2024
12 checks passed
@ngxson
Copy link
Collaborator

ngxson commented Nov 10, 2024

Side note, I'm not very confident that the settings are being categorized in a correct manner (i.e. DRY should be re-grouped to its own section?)

Not a big issue though, suggestion are welcomed for that part.

@MaggotHATE
Copy link
Contributor Author

Previous versions of server UI didn't have any such grouping, only "More options/Further options", I think it's good for now because it serves the purpose and looks uniform with the rest of UI. Given the limited space (it's probably not a good idea to have a looooong settings menu from the beginning) we might have to introduce smaller UI elements with more densely packed information in them (like more granular grouping).

arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 15, 2024
* Add back samplers to server

* Added tooltips with basic information

* Fixed stretching of input fields.

* use component for settings input, move help msg to tooltips

---------

Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
arthw pushed a commit to arthw/llama.cpp that referenced this pull request Nov 18, 2024
* Add back samplers to server

* Added tooltips with basic information

* Fixed stretching of input fields.

* use component for settings input, move help msg to tooltips

---------

Co-authored-by: Xuan Son Nguyen <son@huggingface.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants