Optimizer parameters validation improvement

[himkwtn]

Right now, parameters validation is done in the constructor as follows

pysindy/pysindy/optimizers/sr3.py

Lines 138 to 163 in 3e8a445

	def __init__(
	self,
	threshold=0.1,
	thresholds=None,
	nu=1.0,
	tol=1e-5,
	thresholder="L0",
	trimming_fraction=0.0,
	trimming_step_size=1.0,
	max_iter=30,
	copy_X=True,
	initial_guess=None,
	normalize_columns=False,
	verbose=False,
	unbias=False,
	):
	super(SR3, self).__init__(
	max_iter=max_iter,
	initial_guess=initial_guess,
	copy_X=copy_X,
	normalize_columns=normalize_columns,
	unbias=unbias,
	)
	if threshold < 0:
	raise ValueError("threshold cannot be negative")

According to the scikit-learn documents, validations should be done in the fit method because if we call set_params, it will bypass the validation in the constructor.

Reproducing code example:

from pysindy.optimizers import SR3
opt = SR3(threshold=-1)
# raises "ValueError: threshold cannot be negative"

from pysindy.optimizers import SR3
opt = SR3()
opt.set_params(threshold=-1)
# no error

[Jacob-Stevens-Haas]

This is a good point, although fairly low impact, since I believe set_params() is only used in gridsearch, which is not done much with SINDy, and we can cheat by doing the same validation in set_params() that we do in __init__(). That said, this is something we want, and the way to do this is:

• list out all classes that break the scikit-learn API in differentiation, feature library, and optimizer
• extract all validation into a helper function, refactored to the end of __init__()
• Modify tests for bad argument combinations to require __init__ to pass, and then
• move the validation from __init__ to fit()