search

dynamicslab / pysindy

A package for the sparse identification of nonlinear dynamical systems from data
https://pysindy.readthedocs.io/en/latest/
Other
1.42k stars 310 forks source link

Move `feature_names` from `SINDy.__init__()` to `SINDy.fit()` #387

Open Jacob-Stevens-Haas opened 1 year ago

Jacob-Stevens-Haas commented 1 year ago

Is your feature request related to a problem? Please describe.

If I build a model (model1) and fit it on the first 3 POD modes of a PDE, then I run model2=sklearn.model.clone(model1) and fit on the first 6 modes of a PDE, I should get an error because self.feature_names would be the wrong length. Similarly to #386, feature_names is a data-dependent parameter.

This came up in some of the work I was doing for Nathan.

Describe the solution you'd like

See title

Describe alternatives you've considered

Status quo. Not terrible, it just means explicitly re-creating a model (and feature_library, differentiation_method, and optimizer) rather than being able to simply deep-clone it's state prior to fit().

Additional context

This one's a bit less obvious than #386 since there's no overarching zen to achieve; rather, it's about what sklearn API compliance can achieve.

Jacob-Stevens-Haas commented 11 months ago

Actually this is a little bit confusing. Currently identifying the feature names for the model requires feature_library.fit(). Not sure how this is working right now That may be worth changing, since knowing the ordering of the features is required for setting ConstrainedSR3 constraints. See #422