-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
React UI - Improves code structure, comments, formatting, linting #638
Conversation
Small addition to this PR: "deleting" an image from the web ui now sends it to trash using Previously, when you deleted an image, the server permanently and immediately deleted it. I accidentally deleted some images I wanted to keep and found this package. I've tested it on macOS and it does what it says on the tin. @Kyle0654 this is probably a nice feature for your API. |
Nice. I bet you could switch the API to use it pretty easily (so I don't forget) if you'd like =). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of nice changes to improve code readability here. Thank you. I will do a round of testing before merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The web server doesn't seem to be able to correctly parse the old-style metadata produced by the CLI and behaves unpredictably when trying to load those images. The differences are subtle. Here is the stored prompt produced by the dream.py script:
"blue fire hydrant" -s 50 -W 512 -H 512 -C 7.5 -A k_lms -S 2279065417
and here is the equivalent produced by the flask server:
"blue fire hydrant"-s 50 -S 1954966393 -W 512 -H 512 -C 7.5 -A k_lms
The order of the switches shouldn't matter. I think that @Kyle0654 's metadata parser will handle this correctly.
If you want to make the transition to RFC metadata, here is code that you can call as soon as I do another development check in:
from ldm.dream.args import metadata_loads()
opts = metadata_loads('/path/to/file')
This will return a list of opt objects that look and act like Namespaces:
vars(opts[0])
{
"cfg_scale": 7.5,
"prompt": "a colorful fire hydrant:1.0",
"height": 512,
"steps": 50,
"width": 512,
"strength": 0.75,
"seed": 1387204088,
"postprocessing": [],
"sampler": "k_lms",
"variations": 1387204088:0.15,
"type": "txt2img"
}
"images" is a list because the grid files contain multiple images. Not sure if you want to deal with those at this point.
There are additional functions in ldm.dream.args to write out the metadata in RFC format.
Ultimately this work will likely be superseded by @Kyle0654 's API, but it would be nice for things to work right in the interim.
@psychedelicious here's current way to load and retrieve metadata: Retrieving:
Storing:
metadata_dumps() will ignore the opt.seed and use the provided one if the seeds argument is provided. This is because you usually want to use the autogenerated seed for the generated image rather than the fixed one. The hash of the weights file is stored in the Generate object's model_hash attribute, and is required to be written into the png file by the RFC. If you find anything unsatisfactory about this API let me know. (you too @Kyle0654 !) |
With dependency injection the best approach is to assume that any component you write will be replaceable. The one thing I'd recommend is not assuming you'll be writing to a file. I don't know if you can write to a stream or anything (I'm a lot more used to C#), but I'd at least assume you might want to write to cloud storage 🙂 |
Also I'd prefer to keep a model class for the metadata with simple string serialization/deserialization and strong type hinting. Makes it easier to work with. |
I don't think there's any assumption that the metadata is stored in a file. The serialization/deserialization acts on in-memory objects, including the hashing of the init image which could be a file or a base64 web POST. I think the main design weakness is that the metadata is tied to the RFC, so adding support for features not covered by the RFC will end up making a lot of use of the "extra" field. |
@lstein I wrote my on Concerning that the seed is different the generated REPL command. Is this due to the bug in #641 perhaps? My code takes the seed directly from |
The latest commit represents an initial pass at cleaning up the state and API aspects of the frontend, resulting in friendlier code:
A number of non-code-quality changes were made/features added along the way, at least these:
@lstein noted a bug related to seeds and metadata, once we figure out what action to take and I sort whatever needs sorting, I don't anticipate any further changes (except of course if other bugs are found/requests made). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll just go ahead and change the DREAM prompt that the CLI produces to match the order of arguments that the Web backend expects.
I'm still using my own "parameter to dream prompt" function while I should use the new Args module to take the parameters and make a prompt string. I will make the change today. No need to change the output of the CLI |
This PR represents a first pass at tightening up the new web UI's code structure, comments and formatting. It also resolves some outstanding eslint warnings and a couple minor bugs. There should be no user-facing changes beyond some slightly different component styling.
The most substantial changes are improvements to component modularity (composition? dunno the word). Hopefully the code is more approachable than it was before.
Somewhere along the line, my editor decided to use 2 spaces instead of 4 for indentation and refused to change back, so I just rolled with it.