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

Add Validation loss for LoRA training #1858

Closed
wants to merge 4 commits into from

Conversation

hinablue
Copy link

@hinablue hinablue commented Dec 27, 2024

Base on #1165 @gesen2egee and #914 "Add validation loss" by @rockerBOO works.

And I only test on Lora training, DreamBooth and ControlNet training need more test.

截圖 2024-12-27 16 35 01

@rockerBOO
Copy link
Contributor

instead of taking the code and making a commit of your own you should follow the full commit process so the credit and copyright is property accounted for. It's very rude to all those before it. The PR you took from also did it to my original code and it's really annoying as I get no credit

@hinablue
Copy link
Author

hinablue commented Jan 2, 2025

instead of taking the code and making a commit of your own you should follow the full commit process so the credit and copyright is property accounted for. It's very rude to all those before it. The PR you took from also did it to my original code and it's really annoying as I get no credit

Sorry for the late reply. I've included the original PR link with your information. If you think there is anything that needs to be corrected, please let me know. In fact, I have been following this PR for a long time, and I apologize for the trouble it has caused you.

@kohya-ss
Copy link
Owner

kohya-ss commented Jan 2, 2025

I apologize for not being able to merge the previous validation loss PR. I would like to take advantage of the current holiday season to merge.

@hinablue What is the update from the previous PR?

@rockerBOO Is this PR already okay with you now?

@hinablue
Copy link
Author

hinablue commented Jan 3, 2025

I apologize for not being able to merge the previous validation loss PR. I would like to take advantage of the current holiday season to merge.

@hinablue What is the update from the previous PR?

@rockerBOO Is this PR already okay with you now?

I ref the #1165 @gesen2egee and rewrite it to work in the sd3 branch. Modified the calculation method of val loss after discussing with @gesen2egee (62164e5).

@hinablue
Copy link
Author

hinablue commented Jan 3, 2025

Sorry to disturb everyone, I decided to close this PR. I will find another time to re-make the PR from @rockerBOO 's repo and send it to his sd3 branch.

@hinablue hinablue closed this Jan 3, 2025
@rockerBOO
Copy link
Contributor

@hinablue i'm working to merge these together with the proper commits. Should be ready to review in a couple hours.

@rockerBOO rockerBOO mentioned this pull request Jan 3, 2025
@rockerBOO
Copy link
Contributor

@hinablue Also want to apologize as I was a little tough with my statement previously.

I hope my combination of PR's will be adequate and I appreciate the work you did to fix any conflicts and update the code for the latest. Made the process pretty straight forward for me. I will make sure you get the appropriate credit on the official release, as well as your commits are preserved.

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.

3 participants