Open rockerBOO opened 1 year ago
A possibility to solve the timestep type issue Is to have inside the trait a type
, say timestep_t
which requires to be cloned, copied and allowed to be converted to a primitive type
type timestep_t = Copy + Clone + toPrimitive;
and then use it in the trait as Self::timestep_t
. Therefore, when implementing the trait for a particular scheduler one only needs to set it appropriately
impl Scheduler for MyScheduler {
type timestep_t = usize;
}
I did all of this already, but I'm waiting to open a PR because I have 2 more pending and one more soon to be opened implementing the Heun Discrete scheduler 😅
On second thought, I'm not so sure this is a good idea. Not if we want to port other diffusion models, at least. In fact, some of the missing schedulers have a different implementation logic: some don't implement add_noise
, some others have two different kind of steps (the predictive step and the correct step, with different return types), e.g. the stochastic differential equation (SDE) scheduler.
Right now we are adding the schedulers, but it is difficult to work with since swapping the scheduler doesn't work well. This also slows down testing and evaluation of the schedulers, as a separate script needs to be made each time to test the samplers. I also was implementing these into an application and swapping the schedulers wasn't working (due to different types at runtime).
I experimented some in adding a trait, so we can use
impl Scheduler
. Came up with the following, but causes some points of contention.&mut self
for some schedulersf64
andusize
) currentlyusize
orf64
(or maybe other ones?)And then I could do the following (I'm still learning traits):
And/or we could also do a Scheduler enum.
I'm not 100% sure what's the best approach.