support metadata files as CLI arg supplier #81

anthonywu · 2024-10-19T14:21:28Z

Oct 19 - this is a preview of a big change that I think we'll discuss for several days. It probably should be in the (n+1)th release. I expect we'll go back and forth a few times here because the decisions here influence how other subsequent features are done.

Change

Add a new CLI option to consume the run configs (but not the model configs) from .json files. This allows for very long commands with many path args to be easily re-usable for subsequent runs. You can imagine this being useful for anyone experimenting with changing one thing and seeing how that single variable affects the outcome.
Promote the utility of the existing metadata upgrade files to serve as the input for run configs. The configs are simple enough to be handcrafted, but the metadata output is the automatically compatible one we will offer.
There is some metadata schema change here, but it was not consumed anywhere by mflux so I would not consider this compatibility breaking for SemVar purposes.
See more inline comments for todos and design concerns.

UX Demo

What the change allows are commands like:

mflux-generate --model dev --metadata --config-from-metadata <file> - re-generate from yours or other's prior run configs. You can use these as references for checking into team repos, or to use as the control config in a test suite where the before/after of a image gen change can from the same config file defined in a repo.
mflux-generate --model schnell --metadata --config-from-metadata <file from dev run> - re-do the image gen, but using a diff model than the one that saved the config
mflux-generate --model dev --metadata --config-from-metadata <file> --guidance 5.0 - re-genenerate, but change 1 variable
mflux-generate --model dev --metadata --config-from-metadata <file> --quantize 4 - re-generate, but with the lighter quantized model
put it in a for-loop somewhere (shell or python), and each iteration you change one thing. This is where my output namer proposal in proposal: automatic output file naming #80 is relevant, i.e. I don't want to rename things as I iterate through large batches, I would opt into a smart auto-namer in that scenario.

anthonywu

for early discussion - several iterations expected to come from review

anthonywu · 2024-10-19T14:26:52Z

README.md

@@ -152,6 +152,76 @@ mflux-generate --model dev --prompt "Luxury food photograph" --steps 25 --seed 2

 - **`--controlnet-save-canny`** (optional, bool, default: False): If set, saves the Canny edge detection reference image used by ControlNet.

+- **`--config-from-metadata`** or **`-C`** (optional, `str`): [EXPERIMENTAL] Path to a prior file saved via `--metadata`, or a compatible handcrafted config file adhering to the expected args schema.


Discussion in #80 reminded me that we also export exif data to image files, so that opens the possibility that you can pass in an image path here to, and if you pass in an image path that can act as --init-image or --controlnet-image-path implicitly!

Nice idea, I think that would work!

instead of adding Issues noise, I'll mark some todos for upcoming PRs inline here

add --config-from-image option where the Image is --init-image while the exif metadata is the run config

anthonywu · 2024-10-19T14:27:46Z

README.md

+      "type": ["integer", "null"]
+    },
+    "steps": {
+      "type": ["integer", "null"]
+    },
+    "guidance": {
+      "type": ["number", "null"]
+    },
+    "quantize": {
+      "type": ["null", "string"]
+    },
+    "lora_paths": {
+      "type": ["array", "null"],
+      "items": {
+        "type": "string"
+      }
+    },
+    "lora_scales": {
+      "type": ["array", "null"],
+      "items": {
+        "type": "number"
+      }
+    },
+    "prompt": {
+      "type": ["string", "null"]
+    }


for now just the basic image gen + lora args, withholding the controlnet implementation for now, because there are plans to dedupe/consolidate the two classes together so that would cause change in this PR

Great to also added the schema here! Make sense - lets start start with this and update the schema after that refactoring

todo:

add support for --config-from-metadata in controlnet workflow

README.md

src/mflux/config/config.py

anthonywu · 2024-10-19T14:30:27Z

src/mflux/config/runtime_config.py

+    @property
+    def init_image_path(self) -> int:
+        return self.config.init_image_path
+
+    @property
+    def init_image_strength(self) -> int:
+        return self.config.init_image_strength
+


I think we have some anti-patterns in the relation between "runtime config" and "model config", there's a lot of proxying things through nested config objects. I have some refactoring ideas here but for now I want to keep the config structure as-is so I don't bloat the change and add unrelated changes to the actual intent.

Sounds good! I agree with you that it is not the optimal way of structuring things... More generally, it is also a bit unclear what exactly should go in to the config. For example, now the seed is not included there, but one could argue that it is a type of config etc.

todo:

refactor config classes to have clarity on separation of responsibility and scope

src/mflux/post_processing/generated_image.py

anthonywu · 2024-10-19T14:33:52Z

src/mflux/post_processing/generated_image.py

-            "lora_scales": ", ".join([f"{scale:.2f}" for scale in self.lora_scales]) if self.lora_scales else "None",
+            "seed": self.seed,
+            "steps": self.steps,
+            "guidance": self.guidance if ModelConfig.FLUX1_DEV else None,  # only the dev model supports guidance


I noticed the prior implementation and accepted the logic as is, but want to double check this is true: only the dev model supports guidance

Yes, only the dev model supports guidance. Right now, we pass the guidance value down to the lower level classes (regardless of model choice), but at some point deeper down, we do a check like the following:

self.guidance_embedder = GuidanceEmbedder() if model_config == ModelConfig.FLUX1_DEV else None

and the guidance value is subsequently only used in case of "dev"

blocker/todo for #84

guidance should be turned on for dev-like models as well (e.g. Freepik in support HF models - freepik as first example #84)

src/mflux/post_processing/generated_image.py

src/mflux/ui/cli/parsers.py

anthonywu · 2024-10-19T17:13:43Z

src/mflux/ui/cli/parsers.py

        self.add_argument("--controlnet-strength", type=float, default=ui_defaults.CONTROLNET_STRENGTH, help=f"Controls how strongly the control image influences the output image. A value of 0.0 means no influence. (Default is {ui_defaults.CONTROLNET_STRENGTH})")
        self.add_argument("--controlnet-save-canny", action="store_true", help="If set, save the Canny edge detection reference input image.")

+    def add_metadata_config(self) -> None:
+        self.supports_metadata_config = True
+        self.add_argument("--config-from-metadata", "-C", type=Path, required=False, default=argparse.SUPPRESS, help="Re-use the parameters from prior metadata. Params from metadata are secondary to other args you provide.")


perhaps better to call this simply --config where the metadata-derived files are simply one way of supplying the config

@filipstrand what do you think about simplifying the flag to --config and keep -C or maybe generically --config-from-file / -F

I think both have many precedents in CLI tools.

anthonywu · 2024-10-20T01:43:30Z

src/mflux/ui/cli/parsers.py

+        if path_type == "load":
+            group = self.add_mutually_exclusive_group(required=True)
+            group.add_argument("--model", "-m", type=str, choices=ui_defaults.MODEL_CHOICES, help=f"The model to use ({' or '.join(ui_defaults.MODEL_CHOICES)}).")
+            group.add_argument("--path", type=str, default=None, help="Local path for loading a model from disk")
+        else:
+            self.add_argument("--model", "-m", type=str, required=True, choices=ui_defaults.MODEL_CHOICES, help=f"The model to use ({' or '.join(ui_defaults.MODEL_CHOICES)}).")
+            self.add_argument("--path", type=str, required=True, help="Local path for saving a model.")


@filipstrand this diff does 3 things:

important - fixes a real bug introduced in argparser refactor #75 - so you'll need to accept this PR or cherry-pick these changes before release. I'll try to cherry-pick into a smaller fix-it PR, as time allows in the next few days

re-differentiate the two diff meanings of --path, one is for loading, one is the target of mflux-save. In save context it's required, in the load context it's not required. It may be a better idea to rename the save version to --output-path though!

anthonywu · 2024-10-20T03:02:42Z

@filipstrand in this PR I made major changes to the argparser expectations via adding the new config from file feature, so I added a test suite for all the various entry points. If you find any problems in main, we can:

port some of these fixes and tests to main, and we stay in PR to discuss the config loader feature as a 0.5.0 release
you find this PR acceptable as part of 0.4.0 release scope and we go out with the new tests

anthonywu

test results at this checkpoint:

================================================================================== test session starts ==================================================================================
platform darwin -- Python 3.11.9, pytest-8.3.3, pluggy-1.5.0 -- /Users/anthonywu/Desktop/workspace/mflux-ops/.venv/bin/python3
cachedir: .pytest_cache
rootdir: /Users/anthonywu/workspace/mflux-ops
configfile: pyproject.toml
testpaths: tests
plugins: anyio-4.6.0, timer-1.0.0
collected 17 items
run-last-failure: rerun previous 2 failures first

tests/test_generate_image_controlnet.py::TestImageGeneratorControlnet::test_image_generation_dev_controlnet PASSED                                                                [  5%]
tests/test_generate_image_controlnet.py::TestImageGeneratorControlnet::test_image_generation_dev_lora_controlnet PASSED                                                           [ 11%]
tests/test_cli_argparser.py::test_model_path_requires_model_arg PASSED                                                                                                            [ 17%]
tests/test_cli_argparser.py::test_prompt_arg PASSED                                                                                                                               [ 23%]
tests/test_cli_argparser.py::test_guidance_arg PASSED                                                                                                                             [ 29%]
tests/test_cli_argparser.py::test_quantize_arg PASSED                                                                                                                             [ 35%]
tests/test_cli_argparser.py::test_seed_arg PASSED                                                                                                                                 [ 41%]
tests/test_cli_argparser.py::test_steps_arg PASSED                                                                                                                                [ 47%]
tests/test_cli_argparser.py::test_lora_args PASSED                                                                                                                                [ 52%]
tests/test_cli_argparser.py::test_image_to_image_args PASSED                                                                                                                      [ 58%]
tests/test_cli_argparser.py::test_controlnet_args PASSED                                                                                                                          [ 64%]
tests/test_cli_argparser.py::test_save_args PASSED                                                                                                                                [ 70%]
tests/test_generate_image.py::TestImageGenerator::test_image_generation_schnell PASSED                                                                                            [ 76%]
tests/test_generate_image.py::TestImageGenerator::test_image_generation_dev PASSED                                                                                                [ 82%]
tests/test_generate_image.py::TestImageGenerator::test_image_generation_dev_lora PASSED                                                                                           [ 88%]
tests/test_generate_image.py::TestImageGenerator::test_image_generation_dev_image_to_image PASSED                                                                                 [ 94%]
tests/test_generate_image_controlnet.py::TestImageGeneratorControlnet::test_image_generation_schnell_controlnet PASSED                                                            [100%]

===================================================================================== pytest-timer ======================================================================================
[success] 22.17% tests/test_generate_image_controlnet.py::TestImageGeneratorControlnet::test_image_generation_dev_lora_controlnet: 152.2725s
[success] 22.05% tests/test_generate_image_controlnet.py::TestImageGeneratorControlnet::test_image_generation_dev_controlnet: 151.4941s
[success] 16.29% tests/test_generate_image.py::TestImageGenerator::test_image_generation_dev: 111.8881s
[success] 16.09% tests/test_generate_image.py::TestImageGenerator::test_image_generation_dev_lora: 110.5325s
[success] 15.10% tests/test_generate_image.py::TestImageGenerator::test_image_generation_dev_image_to_image: 103.7386s
[success] 5.61% tests/test_generate_image_controlnet.py::TestImageGeneratorControlnet::test_image_generation_schnell_controlnet: 38.5621s
[success] 2.69% tests/test_generate_image.py::TestImageGenerator::test_image_generation_schnell: 18.5004s
[success] 0.00% tests/test_cli_argparser.py::test_model_path_requires_model_arg: 0.0006s
[success] 0.00% tests/test_cli_argparser.py::test_guidance_arg: 0.0004s
[success] 0.00% tests/test_cli_argparser.py::test_lora_args: 0.0004s
[success] 0.00% tests/test_cli_argparser.py::test_prompt_arg: 0.0004s
[success] 0.00% tests/test_cli_argparser.py::test_image_to_image_args: 0.0004s
[success] 0.00% tests/test_cli_argparser.py::test_quantize_arg: 0.0003s
[success] 0.00% tests/test_cli_argparser.py::test_seed_arg: 0.0003s
[success] 0.00% tests/test_cli_argparser.py::test_steps_arg: 0.0003s
[success] 0.00% tests/test_cli_argparser.py::test_controlnet_args: 0.0003s
[success] 0.00% tests/test_cli_argparser.py::test_save_args: 0.0002s
============================================================================ 17 passed in 688.04s (0:11:28) =============================================================================

pyproject.toml

src/mflux/controlnet/flux_controlnet.py

README.md

tests/test_cli_argparser.py

src/mflux/config/runtime_config.py

README.md

filipstrand · 2024-10-26T15:56:41Z

@anthonywu I think this PR is looking really good and this is a nice feature that takes advantage of the metadata files - I initially just added them in case it was tricky for people to read the metadata embedded in the generated images themselves, and I was thinking that maybe it was a bit of a redundant feature to have, but now it feels like they have a proper purpose!

Just had some smaller comments on this PR, if you mind having a look again. After that, I think we can merge this and do a 0.4.0 release (I'll just add a img2img example in the readme after this one is merged).

Btw, I sent you an invite as a formal collaborator. Really appreciate your continuous involvement with this project!!

…, add tests

anthonywu commented Oct 19, 2024

View reviewed changes

anthonywu force-pushed the regen-from-metadata branch from 6d5d213 to c9dc66e Compare October 19, 2024 23:54

anthonywu commented Oct 19, 2024

View reviewed changes

anthonywu force-pushed the regen-from-metadata branch from c9dc66e to 03c5846 Compare October 19, 2024 23:56

anthonywu commented Oct 20, 2024

View reviewed changes

anthonywu force-pushed the regen-from-metadata branch 2 times, most recently from 5bd1259 to 7a31241 Compare October 20, 2024 02:57

anthonywu mentioned this pull request Oct 20, 2024

tests and fixes for image to image #82

Merged

anthonywu force-pushed the regen-from-metadata branch from 7a31241 to dc61809 Compare October 20, 2024 15:55

support metadata files as CLI arg supplier

351cc8b

anthonywu force-pushed the regen-from-metadata branch from dc61809 to 351cc8b Compare October 20, 2024 16:18

anthonywu commented Oct 20, 2024

View reviewed changes

pyproject.toml Show resolved Hide resolved

src/mflux/controlnet/flux_controlnet.py Outdated Show resolved Hide resolved