-
Notifications
You must be signed in to change notification settings - Fork 147
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
[ENH] Parameterise the cost function of dtw
and adtw
distance measures
#1876
[ENH] Parameterise the cost function of dtw
and adtw
distance measures
#1876
Conversation
Thank you for contributing to
|
dtw
and adtw
distance measuresdtw
and adtw
distance measures
maybe these can go in distance module as parameter to the original dtw based functions ? not saying in this PR as i know its only used for PF2 for now, but future perspective, what do you think @chrisholder ? however we should have a discussion on how it can affect BA stuff (some math related stuff if we go into this as a general parameter) |
FYI there were some large conversations regarding this in the core_devs and distances Slack channels. It is not really compatible with the way those functions are currently set up, best to take a look at there. For now we will do it like this just to move things forward. |
I assume these are the same functions as your previous changes, but just in a new fine @itsdivya1309? If so I dont really see any issue for now. |
Yes, that's right. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it looks fine for now, but I would put it in the same file as PF 2.0 rather than its own.
replaced by #1978 |
Reference Issues/PRs
Closes: #1874
What does this implement/fix? Explain your changes.
As we discussed on the slack, I've implemented the parameterised DTW and ADTW as private functions to be used in the upcoming Proximity Forest 2.0.
Does your contribution introduce a new dependency? If yes, which one?
Any other comments?
PR checklist
For all contributions
For new estimators and functions
__maintainer__
at the top of relevant files and want to be contacted regarding its maintenance. Unmaintained files may be removed. This is for the full file, and you should not add yourself if you are just making minor changes or do not want to help maintain its contents.For developers with write access