[Do Not Merge] Simple Maxdiffusion SDXL inference integration #299
Conversation
mailvijayasingh
requested review from
yeandy,
vipannalla,
RissyRan,
will-cromar and
allenwang28
as code owners
| # Configure flags | ||
| """ python -m src.maxdiffusion.generate_sdxl src/maxdiffusion/configs/base_xl.yml run_name="my_run" """, | ||
| # Give server time to start | ||
| f"sleep {model_configs['sleep_time']}", |
There was a problem hiding this comment.
Is this needed? I had to have this for my jetengine server
There was a problem hiding this comment.
I can delete it.
| "SDXL-Base-1": { | ||
| "sleep_time": 120, | ||
| "tpu_version_cores": [(TpuVersion.V5E, 8), (TpuVersion.V5P, 8)], | ||
| "maxdiffusion_logs": "gs://inference-benchmarks/models/SDXL-Base-1/2024-05-14-14-01/", |
There was a problem hiding this comment.
We can remove since this is unused.
| model_configs = {} | ||
| model_configs["model_name"] = model | ||
| model_configs["sleep_time"] = sweep_model_configs["sleep_time"] | ||
| model_configs["maxdiffusion_logs"] = sweep_model_configs["maxdiffusion_logs"] |
There was a problem hiding this comment.
Not all parameters, but some are used in next revision
mailvijayasingh
changed the title
| @@ -47,10 +56,13 @@ def get_maxdiffusion_inference_nightly_config( | |||
| dataset_name=metric_config.DatasetOption.BENCHMARK_DATASET, | |||
| ) | |||
|
|
|||
| per_device_bat_size = model_configs["per_device_batch_size"] | |||
There was a problem hiding this comment.
| per_device_bat_size = model_configs["per_device_batch_size"] | |
| per_device_batch_size = model_configs["per_device_batch_size"] |
| "pip3 install ." | ||
| "pip3 install .", | ||
| # dependency for controlnet | ||
| "apt-get install ffmpeg libsm6 libxext6 -y" "cd ..", |
There was a problem hiding this comment.
| "apt-get install ffmpeg libsm6 libxext6 -y" "cd ..", | |
| "apt-get install ffmpeg libsm6 libxext6 -y", | |
| "cd ..", |
| "cd ..", | ||
| f"gsutil cp metrics.json {metric_config.SshEnvVars.GCS_OUTPUT.value}", | ||
| ) | ||
| if model_name == "SDXL-Lightning": |
There was a problem hiding this comment.
Suggestion: change to elif here and below.
| "cd maxdiffusion", | ||
| # Configure flags | ||
| "cd .." | ||
| f""" python -m src.maxdiffusion.generate_sdxl src/maxdiffusion/configs/base_xl.yml run_name="my_run" per_device_batch_size={per_device_bat_size} attention="{attention}" """, |
There was a problem hiding this comment.
nit: rename "my_run" to something more specific to "sdxl". Here, and below.
|
|
||
| _modify_save_metrics("metrics.json", model_configs) |
There was a problem hiding this comment.
I'm not sure if _modify_save_metrics will actually affect the metrics.json file. We're running the _modify_save_metrics function when we define the DAG, but to actually need the function to run in the TPU VM at run time.
We can either have bash commands to do something like _modify_save_metrics, which may be more difficult. OR, we could have a python script in the maxdiffusion repo (or other repo) to do this logic. OR, we could do this logic directly in the generate_sdxl.py file (with a flag to add the dimensions key.
There was a problem hiding this comment.
You didn't make these changes right? Can we rebase master please?
|
Looks good overall, can you paste the composer link? |
Description
Simple Integration of Maxdiffusion Inference in auto benchmarking framework
Tests
Tested the flow in dev environment
Please describe the tests that you ran on Cloud VM to verify changes.
Instruction and/or command lines to reproduce your tests: ...
List links for your tests (use go/shortn-gen for any internal link): ...
Checklist
Before submitting this PR, please make sure (put X in square brackets):