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 support for whitespace after the number in constructions like [fo… #12177

Conversation

rubberbaron
Copy link

Description

  • simple description of what the PR tries to accomplish:
    • in prompts like "[foo : 0.5]", whitespace is allowed before the number, but not after the number.
      • if there is whitespace after the number, the string will simply be added to the final prompt without doing prompt editing, and the user can't easily tell this is happening
      • this PR adds support for having whitespace after the number,
    • in prompts like "(foo : 0.5)", whitespace is not allowed before or after the number
      • if there is whitespace before or after the number, the prompt text and number will simply be added to the final prompt with 1.1 emphasis, so e.g. you end up with ["foo : 0.5", 1.1]
      • this PR adds support for having whitespace before and after the number,
  • a summary of changes in code
    • One lark prompt parser rule is expanded to allow whitespace after the number in alternations.
    • The lark visitors handle the extra element in the lark rule
    • The regular expression for numbers in the attention regexp allow whitespace before and after
    • The existing code float(weight) correctly handles whitespace
  • fixes issue I reported: [Bug]: prompt parser doesn't allow whitespace between number and "]" in prompt editing #11627

Screenshots/videos:

Checklist:

@AUTOMATIC1111
Copy link
Owner

The downsides are:

  • this changes seeds
  • i do not like how the thing looks with space after colon

I personally do not want to merge this in, but let's keep the PR open and see what others say.

@nothings
Copy link

The "changes seeds" is a good point!

It's very confusing that the space is allowed before but not after, I'm not sure why the parser was written that way, and I've gotten repeatedly bit by putting spaces in and not being able to figure out why things weren't working right, since there's just no feedback about it. Unfortunately, fixing in the other direction (dropping support for space before numbers in square backets so it's consistent) would also change seeds.

What about a PR that detects cases with spaces in these places and just warns that it may not be doing what you expect?

@AUTOMATIC1111
Copy link
Owner

What about a PR that detects cases with spaces in these places and just warns that it may not be doing what you expect?

Could be good - and this may be possible to do just clientside, if so, the UI is already there, just gotta extend the checker to produce additional warning.

firefox_ZhvX30mowd

@rubberbaron
Copy link
Author

For me, this comes up from doing stuff with wildcards; I have an extended wildcards that lets me specify random ranges like (foo:@1.0-1.3) or [foo:bar:@0.2-0.4], and then those may actually be inside wildcard files, and as the prompts get more complex I may find it more readable to put spaces around it. But that means clientside analysis doesn't really help.

Another scenario where this comes up is when I want to take an existing prompt and seed and keep that as a starting point, and then go in another direction part way through. While I can prompt edit it piecemeal, sometimes I just do:

[
  the original prompt, whatever it was, which is very long, maybe multiple lines
:
  the new prompt, which varies in a number of ways and may have wildcards in it and other complexity
:0.25]

And that works, but one way i tend to try to format that breaks is something like

[
  the original prompt, whatever it was, which is very long, maybe multiple lines
  :the new prompt, which varies in a number of ways and may have wildcards in it and other complexity
  :0.25
]

which clientside testing would catch, but I really want it to work.

That being said, I don't really mind if this fix isn't accepted, I'll just keep the fix locally. Or maybe to avoid conflicts I'll just have my wildcards extension do a final pass over the prompt and clean up spaces around the final numbers.

@AUTOMATIC1111 AUTOMATIC1111 merged commit a1eb496 into AUTOMATIC1111:dev Jul 30, 2023
3 checks passed
@w-e-w w-e-w mentioned this pull request Aug 24, 2023
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.

3 participants