Skip to content

Conversation

@lstein
Copy link
Collaborator

@lstein lstein commented Aug 14, 2023

What type of PR is this? (check all applicable)

  • Bug Fix

Have you discussed this change with the InvokeAI team?

  • Yes

Have you updated all relevant documentation?

  • Yes

Description

On Windows systems, model merging was crashing at the very last step with an error related to not being able to serialize a WindowsPath object. I have converted the path that is passed to save_pretrained into a string, which I believe will solve the problem.

Note that I had to rebuild the web frontend and add it to the PR in order to test on my Windows VM which does not have the full node stack installed due to space limitations.

Related Tickets & Documents

https://discord.com/channels/1020123559063990373/1042475531079262378/1140680788954861698

@blessedcoolant
Copy link
Collaborator

Wont this resolved by just passing dump_path.as_posix() to save_pretrained instead of doing the whole string conversion everywhere else?

@lstein
Copy link
Collaborator Author

lstein commented Aug 15, 2023

Oooh, I didn't know about as_posix() ! I'd been looking for the method that turns paths into strings, but the name must have thrown me.

There are actually two places where paths need to be turned into strings, one just before save_pretrained() and the other where the paths to the models to be merged are passed to the merge pipeline.

I'll go back and restore the call signatures to the way they were, then use as_posix() before passing the paths to the merge code.

@hipsterusername hipsterusername force-pushed the bugfix/merge-crash-on-windows branch from 8597930 to b2934be Compare August 15, 2023 02:59
@blessedcoolant blessedcoolant merged commit e54355f into main Aug 15, 2023
@blessedcoolant blessedcoolant deleted the bugfix/merge-crash-on-windows branch August 15, 2023 03:11
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