Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should we add the firstdiff parameter in sktime too? #13

Open
fkiraly opened this issue Sep 9, 2023 · 2 comments
Open

should we add the firstdiff parameter in sktime too? #13

fkiraly opened this issue Sep 9, 2023 · 2 comments

Comments

@fkiraly
Copy link

fkiraly commented Sep 9, 2023

@lnthach, quick question: should we add the firstdiff parameter in sktime too?

You could make a PR to change the interface class, since changes in parameters do not propagate through the interface.

There's also the issue that the firstdiff parameter is only availale from 0.0.4 on, so we need to take care that users with lower versions don't suddenly error out. (only relevant if we change the interface in sktime)

Also, quick question: not sure whether there is a need to add this in mrsqm - you can easily get the same logic from the pipeline

from sktime.classification.shapelet_based import MrSQM
from sktime.transformations.series.difference import Differencer

sqm = MrSQM(some_params)
first_diff = Differencer()

sqm_with_first_diff = first_diff * sqm

:-)

Of course you decide which parameterizations you want to ship with your estimator, e.g., for convenience, or as a scientific implementation reference to a paper. Just pointing out that users of pipelines can get this (and similar) functionality already.

@lnthach
Copy link
Collaborator

lnthach commented Oct 4, 2023

@fkiraly first_diff (changed from firstdiff) behaves a bit differently though so I am not sure what is the best way to handle this parameter in sktime interface. In mrsqm, if first_diff is True then the features can be extracted from either the raw time series data or first difference (the chance is roughly 50%). This is, I believe, similarly to how some other classifiers (e.g. MultiRocket, Muse, Weasel 2) use the first difference. If first_diff is False then only raw data is used. A closer approximation with sktime would be something like sqm_with_first_diff = sqm + first_diff * sqm.

I am leaning toward including it in sktime so users can have more control. However it needs to be explained clearly to avoid confusion. I can add that in my notebook example. Do you think this works better ?

@fkiraly
Copy link
Author

fkiraly commented Dec 23, 2023

sure, that would work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants