Is your feature request related to a problem? Please describe.
t_default feels a bit of an awkward convention to have two different arguments to specify time, in violation of ZOP 13. It's like saying "Here's the timestep in case I forget to pass it later." But personally, I just end up forgetting which function t is supposed to go in. Less importantly, it goes against the scikit-learn rule that data-dependent parameters go in fit() and not __init__(). To wit, nobody would gridsearch on t_default since training data always has a specific timestep.
Describe the solution you'd like
Remove t_default and simply require t as an argument in fit() and score(). (it doesn't affect predict(), and is required explicitly in simulate)
Describe alternatives you've considered
Status quo. I get that t_default is useful for setting up examples by allowing you to pass an argument to SINDy
model = SINDy(t_default = .1)
This means that you don't need to pass t to fit(), and score()
Additional context
At the end of the day, I'm always a fan of simplifying code, even though it's small. Removing a duplicated feature is a win in my book.
Is your feature request related to a problem? Please describe.
t_defaultfeels a bit of an awkward convention to have two different arguments to specify time, in violation of ZOP 13. It's like saying "Here's the timestep in case I forget to pass it later." But personally, I just end up forgetting which functiontis supposed to go in. Less importantly, it goes against the scikit-learn rule that data-dependent parameters go infit()and not__init__(). To wit, nobody would gridsearch ont_defaultsince training data always has a specific timestep.Describe the solution you'd like
Remove
t_defaultand simply requiretas an argument infit()andscore(). (it doesn't affectpredict(), and is required explicitly insimulate)Describe alternatives you've considered
Status quo. I get that
t_defaultis useful for setting up examples by allowing you to pass an argument toSINDyThis means that you don't need to pass
ttofit(), andscore()Additional context
At the end of the day, I'm always a fan of simplifying code, even though it's small. Removing a duplicated feature is a win in my book.