support flux example #1073
support flux example #1073
Conversation
WalkthroughThe recent updates enhance flexibility and usability across several scripts. Key improvements include new command-line arguments in the image generation script, the introduction of a README for the FLUX model, and optimizations in module availability checks. These changes streamline user interactions with the scripts, allowing for better control over model parameters and environment setup, ultimately leading to improved performance and ease of use. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant LoadPipe
participant Model
User->>CLI: Execute command with parameters
CLI->>LoadPipe: Pass parameters (dtype, revision, local_files_only)
LoadPipe->>Model: Load model with given parameters
Model-->>LoadPipe: Model ready
LoadPipe-->>CLI: Pipeline loaded
CLI-->>User: Execution complete
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (5)
onediff_diffusers_extensions/examples/text_to_image_flux.py (1)
56-56: Avoid unused loop variablei.The loop variable
iis not used within the loop body. Consider using_to indicate that it is intentionally unused.-for i in range(args.warmup): +for _ in range(args.warmup):Tools
Ruff
56-56: Loop control variable
inot used within loop body(B007)
onediff_diffusers_extensions/examples/flux/README.md (3)
16-16: Use proper markdown for URLs.Replace bare URLs with markdown links for better readability and adherence to markdown best practices.
[Set up onediff](https://github.com/siliconflow/onediff?tab=readme-ov-file#installation) [Set up nexfort backend](https://github.com/siliconflow/onediff/tree/main/src/onediff/infer_compiler/backends/nexfort) [Model version for diffusers](https://huggingface.co/black-forest-labs/FLUX.1-schnell) [HF pipeline](https://github.com/huggingface/diffusers/blob/main/docs/source/en/api/pipelines/flux.md)Also applies to: 19-19, 27-27, 29-29
Tools
Markdownlint
16-16: null
Bare URL used(MD034, no-bare-urls)
23-23: Specify language for fenced code block.Specify the language for the fenced code block to improve syntax highlighting and readability.
```bash pip3 install --upgrade diffusers[torch]<details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 23-23: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </blockquote></details> </details> --- `81-81`: **Correct grammatical number.** The phrase "on different CPU" should be corrected to "on different CPUs" for grammatical accuracy. ```diff - and it varies a lot on different CPU. + and it varies a lot on different CPUs.Tools
LanguageTool
[uncategorized] ~81-~81: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...rence, and it varies a lot on different CPU. ## Dynamic shape for FLUX Run: ```...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
benchmarks/text_to_image.py (1)
Line range hint
49-97:
Validate the--dtypeargument.Consider adding validation for the
--dtypeargument to ensure it corresponds to a valid PyTorch data type. This can prevent runtime errors due to invalid input.parser.add_argument("--dtype", type=str, default="half", choices=["float16", "float32", "float64", "half"])
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- benchmarks/text_to_image.py (8 hunks)
- onediff_diffusers_extensions/examples/flux/README.md (1 hunks)
- onediff_diffusers_extensions/examples/text_to_image_flux.py (1 hunks)
- src/onediff/utils/import_utils.py (2 hunks)
Additional context used
Ruff
onediff_diffusers_extensions/examples/text_to_image_flux.py
4-4:
cv2imported but unusedRemove unused import:
cv2(F401)
5-5:
numpyimported but unusedRemove unused import:
numpy(F401)
9-9:
PIL.Imageimported but unusedRemove unused import:
PIL.Image(F401)
56-56: Loop control variable
inot used within loop body(B007)
LanguageTool
onediff_diffusers_extensions/examples/flux/README.md
[uncategorized] ~81-~81: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...rence, and it varies a lot on different CPU. ## Dynamic shape for FLUX Run: ```...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
Markdownlint
onediff_diffusers_extensions/examples/flux/README.md
16-16: null
Bare URL used(MD034, no-bare-urls)
19-19: null
Bare URL used(MD034, no-bare-urls)
27-27: null
Bare URL used(MD034, no-bare-urls)
29-29: null
Bare URL used(MD034, no-bare-urls)
23-23: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (4)
src/onediff/utils/import_utils.py (2)
26-26: Use lazy initialization for_oneflow_available.The change to initialize
_oneflow_availableasNoneand perform the check when needed is a good optimization for lazy evaluation. This avoids unnecessary module checks.
36-38: Ensure thread safety when using global variables.The use of a global variable
_oneflow_availablecan lead to race conditions in a multithreaded environment. Consider using a lock to ensure thread safety if this function is accessed concurrently.Verification successful
Global variable
_oneflow_availableis not accessed in multithreading/multiprocessing contexts.The global variable
_oneflow_availabledoes not appear to be accessed in parts of the codebase that utilize threading or multiprocessing. This suggests that the risk of race conditions affecting this variable is minimal. No further action is needed regarding thread safety for_oneflow_available.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for multithreading usage that might affect `_oneflow_available`. # Test: Search for threading or multiprocessing usage in the codebase. rg --type python 'threading|multiprocessing'Length of output: 77
Script:
#!/bin/bash # Description: Check for multithreading usage that might affect `_oneflow_available`. # Test: Search for threading or multiprocessing usage in the codebase. rg 'threading|multiprocessing'Length of output: 310
Script:
#!/bin/bash # Description: Check if `_oneflow_available` or related functions are accessed in multithreading/multiprocessing contexts. # Test: Search for `_oneflow_available` references in files using threading or multiprocessing. rg '_oneflow_available' onediff_comfy_nodes/benchmarks/src/core/service_client.py onediff_diffusers_extensions/examples/text_to_image_sdxl_mp_load.py # Test: Search for function calls from `import_utils.py` in files using threading or multiprocessing. rg 'import_utils' onediff_comfy_nodes/benchmarks/src/core/service_client.py onediff_diffusers_extensions/examples/text_to_image_sdxl_mp_load.pyLength of output: 293
benchmarks/text_to_image.py (2)
Line range hint
114-137:
LGTM! Verify usage of new parameters.The changes to include
revisionandlocal_files_onlyare well-integrated. The assertion fordtypeis a good practice.Ensure that the usage of these new parameters is consistent throughout the codebase.
Verification successful
Consistent Usage of
revisionandlocal_files_onlyParametersThe parameters
revisionandlocal_files_onlyare consistently used throughout the codebase, as evidenced by their presence in function calls and argument parsing. This aligns with the changes made in theload_pipefunction.
- Instances of
revisionandlocal_files_onlywere found in argument parsing and function calls, indicating consistent usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `revision` and `local_files_only` parameters in the codebase. # Test: Search for the usage of these parameters in function calls. Expect: Consistent usage. rg --type python -A 5 $'revision|local_files_only'Length of output: 82
Script:
#!/bin/bash # Description: Verify the usage of `revision` and `local_files_only` parameters in the codebase. # Find all Python files and search for the usage of `revision` and `local_files_only` parameters. fd --extension py --exec rg 'revision|local_files_only' {}Length of output: 1197
246-253: Ensure proper integration of new arguments.The changes to pass
dtype,revision, andlocal_files_onlytoload_pipeare well-implemented.Verify that these arguments are correctly integrated and utilized in the pipeline setup.
Verification successful
Integration of
dtype,revision, andlocal_files_onlyis correct.The
dtype,revision, andlocal_files_onlyarguments are properly integrated into the pipeline setup inbenchmarks/text_to_image.py. These arguments are correctly passed to theload_pipefunction and utilized in the pipeline configuration.
dtypeis used to set thetorch_dtype.revisionis added toextra_kwargs.local_files_onlyis handled as a boolean flag.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of `dtype`, `revision`, and `local_files_only` in the pipeline setup. # Test: Search for the integration points in the pipeline setup. Expect: Correct usage and handling. rg --type python -A 5 $'dtype|revision|local_files_only'Length of output: 88
Script:
#!/bin/bash # Description: Verify the integration of `dtype`, `revision`, and `local_files_only` in the pipeline setup. # Test: Search for the integration points in the pipeline setup. Expect: Correct usage and handling. rg --glob '*.py' -A 5 'dtype|revision|local_files_only'Length of output: 214146
| import argparse | ||
| import time | ||
|
|
||
| import cv2 |
There was a problem hiding this comment.
Remove unused imports.
The imports cv2, numpy, and PIL.Image are not used in the script and should be removed to clean up the code.
-import cv2
-import numpy as np
-import PIL.ImageAlso applies to: 5-5, 9-9
Tools
Ruff
4-4:
cv2imported but unusedRemove unused import:
cv2(F401)
| end = time.time() | ||
| print(f"Inference time: {end - begin:.3f}s") | ||
|
|
||
| image.save(f"{i=}th_{args.saved_image}.png") |
There was a problem hiding this comment.
Fix incorrect string interpolation in image.save.
The use of f"{i=}th_{args.saved_image}.png" is incorrect for filename generation. Correct the string formatting to avoid syntax errors.
- image.save(f"{i=}th_{args.saved_image}.png")
+ image.save(f"{i}th_{args.saved_image}.png")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image.save(f"{i=}th_{args.saved_image}.png") | |
| image.save(f"{i}th_{args.saved_image}.png") |
There was a problem hiding this comment.
@jackalcooper，好的，去掉等号是正确的选择。这样文件名会更简洁。感谢您的确认！
| ).images[0] | ||
| end_time = time.time() | ||
| print(f"Inference time: {end_time - start_time:.2f} seconds") | ||
| image.save(f"{i=}th_{args.saved_image}_{h}x{w}.png") |
There was a problem hiding this comment.
Fix incorrect string interpolation in image.save.
The use of f"{i=}th_{args.saved_image}_{h}x{w}.png" is incorrect for filename generation. Correct the string formatting to avoid syntax errors.
- image.save(f"{i=}th_{args.saved_image}_{h}x{w}.png")
+ image.save(f"{i}th_{args.saved_image}_{h}x{w}.png")Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| image.save(f"{i=}th_{args.saved_image}_{h}x{w}.png") | |
| image.save(f"{i}th_{args.saved_image}_{h}x{w}.png") |
There was a problem hiding this comment.
@jackalcooper, 正如建议中所示，我已经去掉了文件名中的等号。请查看是否符合您的要求！
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Summary by CodeRabbit
New Features
Bug Fixes
Documentation