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

GuernikaCore fixes #4

Merged
merged 12 commits into from
Mar 14, 2024
Merged

GuernikaCore fixes #4

merged 12 commits into from
Mar 14, 2024

Conversation

gdbing
Copy link

@gdbing gdbing commented Mar 13, 2024

A number of issues with GuernikaCore were identified in this PR which I have tried to address in this branch

  1. Additions and changes beyond the scope of the PR
    • revert Style Prompt Presets
    • revert changes to GalleryView UI
    • revert project codesigning
    • revert SwiftLint
  2. Assorted code style issues
    • ran SwiftLint
  3. Overwriting model data
    • copy variable size models to a temp folder and modify/load data from there
  4. A number of bugs related to ImageGenerator and ImageController directly accessing shared properties
    • refactored code between classes so that properties were directly accessible to logic that required them
    • refactored code to pass all req data through GenerationConfig

I also made some changes refactoring code to improve clarity.

Overwriting model data

When models are loaded (by ImageController.loadModels() or reloadModels()), if a variable size model is detected, its data is copied to a temp folder and loaded from there instead. Any modifications happen to the data in the temp folder instead of the original model files.

If the user's models folder is in their main volume, the FileManager.temporaryDirectory() folder is used, otherwise a folder is created in the same directory as the user's models folder. The folder is cleaned up by Mochi at quit, or if user changes their models folder in settings.

Since the temp folder is in the same volume as the models folder, it is possible to hard link most of the data, only duplicating the following files

  • VAEEncoder.mlmodelc/coremldata.bin
  • VAEDecoder.mlmodelc/coremldata.bin
  • VAEEncoder.mlmodelc/model.mil
  • VAEDecoder.mlmodelc/model.mil
  • VAEEncoder.mlmodelc/metadata.json

This is fast and efficient with disk space.

Unlike a previous PR I submitted, this makes just one copy of the model and edits it in place, and does not load a different copy of the model into memory for each different shape used.

shared property bugs

Mochi allows users to queue up multiple generations, with different configurations, by storing the configurations in a queue of GenerationConfig objects which are sent from ImageController to ImageGenerator.

ImageGenerator was directly accessing ImageController to get properties using its shared global instance reference instead, causing failures and crashes with queued jobs.

e.g. from ImageGenerator.loadPipeline()

if model.allowsVariableSize && vaeAllowsVariableSize(model.url) == false{
    await modifyInputSize(model.url, height: ImageController.shared.height, width: ImageController.shared.width)
}

In this example the model was being modified with the current shape dimensions from ImageController, which may no longer be the same as the dimensions of ImageController at the time the current queued job was created. If the initImage in the pipelineConfig has dimensions which mismatch the inputSize it will cause a fatal error.

I moved some logic from ImageGenerator to ImageController, so that they wouldn't need to share data.

assorted refactoring

  • Related to the above, logic related to hashing and checking old pipelines has been moved from loadPipeline() in ImageGenerator to runGenerationJobs() in ImageController, and has been combined with the new variable size requirements
  • method calls related to the variable size model hack have also been moved from loadPipeline() and generate() to runGenerationJobs()
  • I think that hackVAE is a very clever bit of functionality, but even when I understood what it was doing I found it very difficult to trace through the multiple nested closures. So I split it into three pure inline functions, resizeableCopy(), modifyEncoderMil(), and modifyDecoderMil() which I think are easier to read through.
  • Tightened up access control. I added private to a bunch of methods and properties which shouldn't be publicly accessible, and turned the variable size hack functions into methods of SDModel

Variable size requirements

I want to explicitly spell out my understanding of what the variable size hack needs because if I am misunderstanding anything I want it to be obvious so that any related bugs can be caught and corrected.

  • VAEEncoder.mlmodelc/coremldata.bin replaced with en-coremldata.bin
  • VAEDecoder.mlmodelc/coremldata.bin replaced with de-coremldata.bin

These just need to happen once, so they get replaced in resizeableCopy() when the model is copied to its temp folder

  • VAEEncoder.mlmodelc/model.mil modified by modifyEncoderMil()
  • VAEDecoder.mlmodelc/model.mil modified by modifyDecoderMil()

model.mils have to be updated every time the size changes, so size is now included in the pipelineHash which gets checked to reload the pipeline

  • VAEEncoder.mlmodelc/metadata.json modified by modifyInputSize()

Input size must be updated when there is an input image or controlnet of a new size. The cache must be emptied to reload the image, so loadPipeline() is called with reduceMemory = true which unloads existing VAEEncoder data

Remaining issues

I have commented out lines 329 and 337 of ImageController.swift

// ImageGenerator.shared.pipeline?.conditioningInput = [cinput]
// ImageGenerator.shared.pipeline?.conditioningInput = [ainput]

They have the same problem of using shared to set properties out of sync with the generation job queue. I am working on a fix but don't understand ControlNets well enough yet and didn't want to hold up submitting these proposed changes any longer.

gdbing added 12 commits March 7, 2024 00:02
- pipeline invalidated by changes to
  - model
  - controlnets
  - computeUnit
  - reduceMemory setting

- the hack which allows variable inputSize requires that the pipeline
  be invalidated when starting image changes size
  - also updates VAEEncoder inputSize
  - also unloads VAEEncoder from memory with reduceMemory = true
- turn functions hackVAE and modifyInputSize into SDModel methods
- include size in pipelineHash
- include presence of inputImage in pipelineHash
- move hackVAE call from ImageGenerator to ImageController
- when models are loaded (or reloaded) variable size models are copied
  to a temp folder and their data is loaded from there, so that Mochi
  can edit files required for variable size image generation without
  affecting the original model data
- model data is hard linked, except for the files which will be edited
- temp folder is created in same volume as models so that models can be 
  hard linked
    - if models folder is in System volume, uses
      FileMonitor.temporaryDirectory
    - otherwise creates temp folder in models/../
- reload models when model folder is changed in settings
- cleanup temp folder on app quit or reloading models
@czkoko czkoko merged commit 1219b1d into czkoko:GuernikaCore Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants