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

Move progress bar to account for sampling image first #1909

Open
wants to merge 1 commit into
base: sd3
Choose a base branch
from

Conversation

rockerBOO
Copy link
Contributor

Progress bar was including sampling images (--sample_at_first) and causing some amount of the time to be off what would be expected.

@stepfunction83
Copy link

stepfunction83 commented Jan 29, 2025

I know that tqdm() is called multiple times in the script for the progress bar to recreate it instead of calling progress_bar.reset(). Have you tried just resetting it instead of recreating it and seeing how that impacts its placement?

The placement on screen is very sensitive to the order that the tqdm() progress bar initializations are called. In #1900 for example, I replace the multiple calls with multiple uses of .reset() to fix the placement on screen.

@rockerBOO
Copy link
Contributor Author

rockerBOO commented Jan 29, 2025

Could have it reset before it goes into training where I moved it. But I think it makes sense to put it where I have it right before the training. The progress is for the training steps so I think it makes sense to move it to capture this specifically.

The other things you improved on validation progress may allow for different results and reset may be more appropriate there.

Edit: This was highlighted when I was testing single epochs times and the sampling was happening so the estimate of time wasn't accurate to just the epoch of training.

@stepfunction83
Copy link

stepfunction83 commented Jan 29, 2025

Oh, I see, you just pushed the first initialization down in the code. That makes perfect sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants