derrian-distro / LoRA_Easy_Training_Scripts

A UI made in Pyside6 to make training LoRA/LoCon and other LoRA type models in sd-scripts easy
GNU General Public License v3.0
998 stars 101 forks source link

Keep Tokens Separator does not work anymore + workaround #190

Closed firsc closed 4 months ago

firsc commented 4 months ago

On the dev branch, the token separator function does not work anymore. I believe it worked on one of the previous commits. Either interacting with this text box (inserting or removing text) or loading in toml files with a given token separator will spit the same error:

...
  File "/home/user/repos/easyscripts2/main_ui_files/GeneralUI.py", line 188, in edit_args
    if elem and elem.dirty and (not elem.allow_empty or elem.text() != ""):
AttributeError: 'LineEditWithHighlightMin' object has no attribute 'allow_empty'

The effects of this are:

If I do a dirty quick fix by replacing not elem.allow_empty with (hasattr(elem, 'allow_empty') and not elem.allow_empty), loading tomls and interacting with the separator text box works again, but submitting will then lead to a new error:

Traceback (most recent call last):
  File "/home/user/miniconda3/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/home/user/miniconda3/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/home/user/repos/easyscripts2/main_ui_files/MainUI.py", line 149, in start_training_thread
    self.train_helper(url, Path("queue_store/temp.toml"))
  File "/home/user/repos/easyscripts2/main_ui_files/MainUI.py", line 166, in train_helper
    print(f"Item Failed: {response.json()}")
  File "/home/user/repos/easyscripts2/venv/lib/python3.10/site-packages/requests/models.py", line 975, in json
    raise RequestsJSONDecodeError(e.msg, e.doc, e.pos)
requests.exceptions.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Seems like the response throws a 500. The response itself is from the POST in line 158 of MainUI.py in f"{url}/validate", json=True, data=json.dumps(final_args), so something in the validate endpoint fails. The response itself is not JSON but an encoded string of a traceback, so response.json() will fail. Replacing this with response.content.decode() reveals the following traceback:

  File "/home/user/repos/easyscripts2/backend/main.py", line 86, in validate_inputs
    passed_validation, sdxl, errors, args, dataset_args, tags = validate(body)
  File "/home/user/repos/easyscripts2/backend/utils/validation.py", line 12, in validate
    args_pass, args_errors, args_data = validate_args(args["args"])
  File "/home/user/repos/easyscripts2/backend/utils/validation.py", line 77, in validate_args
    if arg == "keep_tokens_separator" and len(args[arg]) < 1:
KeyError: 'keep_tokens_separator'

Inside this backend code, replacing len(args[arg]) with len(val) or len(value[arg]) fixes this problem.

To summarize, there were 3 bugs that I found:

  1. LineEditWithHighlightMin which is used by Keep Tokens Separator is missing the 'allow_empty' attribute
  2. The validate backend can return non-JSON, e.g. an encoded traceback string which will trip up the .json() call
  3. The len(args[arg]) trips up in the validator

My intention with the token separator is to train LoRAs which are compatible with NAIv3/Animagine's tag ordering. I'm on WSL, latest dev commit (4bda9fb).

derrian-distro commented 4 months ago

oh thanks for catching that, I meant to remove the check for that one when I removed the rest.

derrian-distro commented 4 months ago

fixed the bug, report back if it worked and then I'll close the issue

firsc commented 4 months ago

Hi, I've tried it again. The first bug has been fixed, but the other two are still there, namely that the traceback response is decoded with response.json() in main_ui_files/MainUI.py:163 (maybe this could be a response.content().decode() instead?) and that within the backend module in backend/utils/validation.py:77 there's len(args[arg]) which is probably supposed to be len(val) to check the length of the keep_tokens_separator string.

derrian-distro commented 4 months ago

it shouldn't be checking the length anymore at all.

derrian-distro commented 4 months ago

so long as validation doesn't crash, it will always output a predicable json format to display what has failed validation, no need to change that, if it crashes, it should crash, so I don't plan on changing that.

I made the change to the validation, you were right, I wasn't checking the correct element, so I made that change

firsc commented 4 months ago

Thanks, appreciate your work! I think this issue can be closed then