Refactor progress bar #242
Conversation
|
Oh, I just noticed #172 (comment). But I guess this is already addressed in my PR? |
| extra_step_kwargs["eta"] = eta | ||
|
|
||
| for i, t in tqdm(enumerate(self.scheduler.timesteps)): | ||
| for i, t in enumerate(self.progress_bar(self.scheduler.timesteps)): |
There was a problem hiding this comment.
As for enumerate and tqdm, if the order is enumerate(tqdm(...)), we don't have to pass total.
There was a problem hiding this comment.
Agree that's what it states in the tqdm docs: https://pypi.org/project/tqdm/#faq-and-known-issues
|
The documentation is not available anymore as the PR was closed or merged. |
src/diffusers/pipeline_utils.py
Outdated
|
|
||
| config_name = "model_index.json" | ||
|
|
||
| def __init__(self): |
There was a problem hiding this comment.
Could we remove the __init__ function here? It's usually much easier to read components when there is no __init__ function :-)
There was a problem hiding this comment.
The main reason here is that adding an __init__ to a class makes the class automatically harder to understand e.g. if you abstract from this class and the class doesn't have an init you're given much more freedom with your "parent" class
| captured = capsys.readouterr() | ||
| assert "10/10" in captured.err, "Progress bar has to be displayed" | ||
|
|
||
| ddpm.set_progress_bar_config(disable=True) |
There was a problem hiding this comment.
simple way to turn off the progress bar
patil-suraj
left a comment
patil-suraj
left a comment
There was a problem hiding this comment.
LGTM, thanks a lot for the PR @hysts ❤️
anton-l
left a comment
anton-l
left a comment
There was a problem hiding this comment.
LGTM, thanks for working on this @hysts @patrickvonplaten
|
|
||
| ddpm = DDPMPipeline(model, scheduler).to(torch_device) | ||
| ddpm(output_type="numpy")["sample"] | ||
| captured = capsys.readouterr() |
There was a problem hiding this comment.
Nice test, gonna remember this ;)
|
Sorry for having fiddled into your PR quite a bit here @hysts - thanks a mille for the contribution ❤️ |
* Refactor progress bar of pipeline __call__ * Make any tqdm configs available * remove init * add some tests * remove file * finish * make style * improve progress bar test Co-authored-by: Patrick von Platen <[email protected]>
* Add debug log of torch_model_blacklist.txt * Add make_fx for torch model * Update torch_model_blacklists.txt * Add some Xfails
* Refactor progress bar of pipeline __call__ * Make any tqdm configs available * remove init * add some tests * remove file * finish * make style * improve progress bar test Co-authored-by: Patrick von Platen <[email protected]>
5 participants
Close #172
Hi, @patrickvonplaten
I think fda6308 is what you initially had in mind. But I think it would be better if we could set other
tqdmparameters as well. It's because I think it would be better to useleave=Falseinstead of disablingtqdmentirely sometimes, especially when using the DDPM scheduler with 1000 steps to generate images.So I suggest 3aa44e8. What do you think?
Usage:
BTW, I'm not familiar with the design of the
diffusersyet and have a question. I added__init__toDiffusionPipelineclass, but is this OK, or is there any other better/supposed way?