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

[ENH] Add Swale Scoring Model #1365

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

aadya940
Copy link
Contributor

@aadya940 aadya940 commented Apr 3, 2024

Related to: #425

Implemented as per this paper, Section 5.

Couldn't verify the correctness due to a lack of alternate implementations. I shall continue to work on this once we track down an alternate implementation?

@aeon-actions-bot aeon-actions-bot bot added distances Distances package enhancement New feature, improvement request or other non-bug code enhancement labels Apr 3, 2024
@aeon-actions-bot
Copy link
Contributor

Thank you for contributing to aeon

I have added the following labels to this PR based on the title: [ $\color{#FEF1BE}{\textsf{enhancement}}$ ].
I have added the following labels to this PR based on the changes made: [ $\color{#5209C9}{\textsf{distances}}$ ]. Feel free to change these if they do not properly represent the PR.

The Checks tab will show the status of our automated tests. You can click on individual test runs in the tab or "Details" in the panel below to see more information if there is a failure.

If our pre-commit code quality check fails, any trivial fixes will automatically be pushed to your PR unless it is a draft.

Don't hesitate to ask questions on the aeon Slack channel if you have any.

@aadya940 aadya940 marked this pull request as draft April 3, 2024 08:53
@MatthewMiddlehurst
Copy link
Member

Is this ready to go? There seems to be a conflict currently. People are less likely to look at drafts 🙂.

@aadya940
Copy link
Contributor Author

@MatthewMiddlehurst From my side this looks ready to go. However, I'm not 100% confident about the logic used. We should check this up against an alternate implementation (which I couldn't track down) if something looks wrong to you. :-)

@aadya940 aadya940 marked this pull request as ready for review April 19, 2024 08:24
@TonyBagnall
Copy link
Contributor

thanks for this, I missed it. I will look for an implementation, probably one in matlab. There is a conflict with test_expected_results though

@TonyBagnall
Copy link
Contributor

so the implementation is true to the definition in the paper, but I dont think defining this recursively is a good idea. It would be better if it followed the model used by LCSS. @chrisholder and I will take a look and get back to you

chrisholder
chrisholder previously approved these changes May 3, 2024
@MatthewMiddlehurst
Copy link
Member

There seems to be a conflict which has to be resolved, but @chrisholder has approved so this should be quick to merge after given him and @TonyBagnall have no issues with it.

@aadya940 aadya940 marked this pull request as draft May 29, 2024 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
distances Distances package enhancement New feature, improvement request or other non-bug code enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants