Conversation
|
Looping in @yiyixuxu here. |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
|
@lawrence-cj let's also add a testing suite here. You can refer to a test suite for schedulers here: |
|
awesome :) |
|
No problem, I will add a test for you. |
|
|
||
| if isinstance(timestep, torch.Tensor): | ||
| timestep = timestep.to(self.timesteps.device) | ||
| step_index = (self.timesteps == timestep).nonzero() |
There was a problem hiding this comment.
Can we add a step_index counter instead of searching for the index at each step?
Like how it is done here for DPM
Essentially we only need to search for the step_index once in the beginning, and then just increase the counter after each step
There was a problem hiding this comment.
We implemented sa-solver according to DPMSolverMultistep in diffusers==0.21.2. Could you please help us to change it to the newest implementation?
There was a problem hiding this comment.
As stated by @yiyixuxu, we need to change the logic to use a self.step_index counter or else this scheduler won't function correctly when using karras sigmas or when doing img2img translation. Could you take a look at
add sa solver test file
| result_mean = torch.mean(torch.abs(sample)) | ||
|
|
||
| if torch_device in ["mps"]: | ||
| assert abs(result_sum.item() - 176.66974135742188) < 1e-2 |
There was a problem hiding this comment.
We have no idea how to get the specific value here.
There was a problem hiding this comment.
Think we can remove the mps check for now as well
| assert abs(result_sum.item() - 167.47821044921875) < 1e-2 | ||
| assert abs(result_mean.item() - 0.2178705964565277) < 1e-3 | ||
| elif torch_device in ["cuda"]: | ||
| assert abs(result_sum.item() - 171.59352111816406) < 1e-2 |
There was a problem hiding this comment.
What about this line? Wondering how to get the specific value 171.59352111816406, etc.
|
There are some tests failing here. Could we please fix them? |
|
I will let @yiyixuxu review this once the test failures are resolved. |
|
Gentle ping here @yiyixuxu |
* fix bugs in repository consistency
fix bugs in init file and sasolverscheduler file
update for code quality check
modify for step_index
|
We add |
make style
fix copy inconsistencies
|
Gentle ping here. @yiyixuxu |
|
Happy to merge whenever @yiyixuxu is happy here! |
|
Thx so much for your review. I'm wondering how you make this style automatically. : ) Any tools for it? @yiyixuxu |
|
@lawrence-cj if you installed our dev environment, you could run |
|
Oh, thank you so much for the information. @yiyixuxu |
|
@lawrence-cj |
* add Sa-Solver --------- Co-authored-by: Sayak Paul <spsayakpaul@gmail.com> Co-authored-by: scxue <xueshuchen17@mails.ucas.edu.cn> Co-authored-by: jschen <chenjunsong4@h-partners.com> Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com> Co-authored-by: yiyixuxu <yixu310@gmail,com>
6 participants
What does this PR do?
Add Sa-Solver into diffusers lib.
Cc: @patil-suraj. Really need your help to test it.