UniPC remove order short-circuit for solve(R, b)

[Beinsezii]

When unifying skrample's UniP and UniC solvers I noticed the rks matrix solve short circuits when it likely shouldn't.

I think it's because it appends to RKS

diffusers/src/diffusers/schedulers/scheduling_unipc_multistep.py

Line 853 in b5c2050

rks.append(1.0)

but only checks by order

diffusers/src/diffusers/schedulers/scheduling_unipc_multistep.py

Lines 886 to 890 in b5c2050

	# for order 1, we use a simplified version
	if order == 1:
	rhos_c = torch.tensor([0.5], dtype=x.dtype, device=device)
	else:
	rhos_c = torch.linalg.solve(R, b).to(device).to(x.dtype)

Reading the paper https://arxiv.org/abs/2302.04867 I don't see any 0.5 short circuits so I'm assuming this was added to stop a 1x1 solve from erring. The comments would imply the results should be equivalent, however because you're skipping actual 2x2 solves the results are quite different for high orders

main

beinsezii/unipc_matrix_solve

cc @yiyixuxu

I'm not a mathematician so it might be prudent to double-check the paper yourself to make sure my homework is correct.

[Beinsezii]

we might not even need the check at all let me see.

[Beinsezii]

Yeah solve() is fine because it does append(1.0)

[chaObserv]

It's in the implementation details of the paper. They use an approximate value 0.5 for UniP-2 and UniC-1 to avoid solving the equation.

[Beinsezii]

It's in the implementation details of the paper. They use an approximate value 0.5 for UniP-2 and UniC-1 to avoid solving the equation.

Hm, it's strange to me that it's off by something like a double digit % vs linalg.solve(). The perf impact of such an op in a denoise is basically immeasurable even for sd15.