Skip to content

[ENH, REF]Refactored time-point based ROCKAD implementation #2804

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 28 commits into from
Jun 19, 2025
Merged

[ENH, REF]Refactored time-point based ROCKAD implementation #2804

merged 28 commits into from
Jun 19, 2025

Conversation

pattplatt
Copy link
Contributor

What does this implement/fix? Explain your changes.

  • Clarified in docstrings and comments that ROCKAD is designed for semi-supervised use
  • Updated tests to reflect semi-supervised behavior (fit() on training data, predict() on test data)
  • Added a new test case
  • Removed predict_proba() and moved its logic to _inner_predict() to improve consistency
  • Added reference and example usage to class-level description

One open question is if we should remove _fit_predict() as it is just misleading for a semi-supervised approach, or should the user be able to use/test it even if ROCKAD is not intended to be used unsupervised?

  • The PR title starts with either [ENH], [MNT], [DOC], [BUG], [REF], [DEP] or [GOV] indicating whether the PR topic is related to enhancement, maintenance, documentation, bugs, refactoring, deprecation or governance.

MatthewMiddlehurst and others added 22 commits November 8, 2024 16:12
@MatthewMiddlehurst
mockup
775e1e8
@MatthewMiddlehurst
init
ba1c55e
@MatthewMiddlehurst
decorator
8eb67c8
@MatthewMiddlehurst
Merge remote-tracking branch 'origin/main' into mm/collection-ad
9762e3c
@MatthewMiddlehurst
correct import and tag
a6d33ab
@MatthewMiddlehurst
Merge remote-tracking branch 'origin/main' into mm/collection-ad
a27ab6d 
# Conflicts:
#	aeon/testing/estimator_checking/_yield_estimator_checks.py
#	aeon/testing/testing_data.py
@MatthewMiddlehurst
base docs
2db96f2
@MatthewMiddlehurst
wrappers
1920ee4
@MatthewMiddlehurst
tests
debc38b
@MatthewMiddlehurst
docs and imports
7179b26
@MatthewMiddlehurst
test params
3d37a81
@MatthewMiddlehurst
register
4ba7692
@MatthewMiddlehurst
Merge remote-tracking branch 'origin/main' into mm/collection-ad
36cf7a9 
# Conflicts:
#	aeon/anomaly_detection/whole_series/__init__.py
#	docs/api_reference/anomaly_detection.rst
@MatthewMiddlehurst
big refactor
d47b537
@MatthewMiddlehurst
smoothing refactor
c245ccb
@MatthewMiddlehurst
Revert "smoothing refactor"
f6122d7 
This reverts commit c245ccb.
@MatthewMiddlehurst
fixes
d777a12
@MatthewMiddlehurst
final bits and docs for refactor
7508ffd
@pattplatt
move predict_proba logic into inner_predict for code consistency
97c5f25
@pattplatt
Added reference and example code, updated description
c5bbc96
@pattplatt
Adapted tests to be semi-supervised
8844eb1
@pattplatt
Added semi-supervised labeling to description
7feab8f
@aeon-actions-bot aeon-actions-bot bot added anomaly detection Anomaly detection package enhancement New feature, improvement request or other non-bug code enhancement refactor Restructuring without changing its external behavior labels May 14, 2025
@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}}$, $\color{#BDD9DB}{\textsf{refactor}}$ ].
I have added the following labels to this PR based on the changes made: [ $\color{#6F6E8D}{\textsf{anomaly detection}}$ ]. 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.

PR CI actions

These checkboxes will add labels to enable/disable CI functionality for this PR. This may not take effect immediately, and a new commit may be required to run the new configuration.

  • Run pre-commit checks for all files
  • Run mypy typecheck tests
  • Run all pytest tests and configurations
  • Run all notebook example tests
  • Run numba-disabled codecov tests
  • Stop automatic pre-commit fixes (always disabled for drafts)
  • Disable numba cache loading
  • Push an empty commit to re-run CI checks

MatthewMiddlehurst
MatthewMiddlehurst reviewed May 15, 2025
View reviewed changes
Copy link
Member

@MatthewMiddlehurst MatthewMiddlehurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One open question is if we should remove _fit_predict() as it is just misleading for a semi-supervised approach, or should the user be able to use/test it even if ROCKAD is not intended to be used unsupervised?

I'm not sure it is misleading unless there is something off with the docs? it fits and predicts on the same data as intended. Sometimes there are more efficient ways to do this than calling both methods individually which is why we allow it to be overloaded.

@MatthewMiddlehurst
Copy link
Member

We do more with tags and testing here #2652

@SebastianSchmidl
Copy link
Member

we should probably wait until #2652 and #2326 are merged to avoid too many conflicts

Base automatically changed from mm/collection-ad to main May 30, 2025 22:01
@review-notebook-app Review Notebook App
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

@MatthewMiddlehurst
Merge remote-tracking branch 'origin/main' into pm/time_point_ROCKAD_…
675669d 
…refactor

# Conflicts:
#	aeon/anomaly_detection/collection/_classification.py
#	aeon/anomaly_detection/collection/_outlier_detection.py
#	aeon/anomaly_detection/collection/base.py
#	aeon/anomaly_detection/series/base.py
#	aeon/anomaly_detection/series/distance_based/__init__.py
#	aeon/anomaly_detection/series/outlier_detection/__init__.py
#	aeon/anomaly_detection/series/outlier_detection/tests/test_one_class_svm.py
#	aeon/testing/estimator_checking/_yield_collection_anomaly_detection_checks.py
#	aeon/testing/estimator_checking/_yield_estimator_checks.py
#	aeon/testing/testing_data.py
#	aeon/utils/base/_identifier.py
#	aeon/utils/base/_register.py
#	aeon/utils/tags/_tags.py
#	docs/api_reference/anomaly_detection.rst
@MatthewMiddlehurst
merge 2
68a2ccb
@MatthewMiddlehurst
CI errors
771dcc9
@MatthewMiddlehurst MatthewMiddlehurst requested a review from a team as a code owner June 18, 2025 10:47
@MatthewMiddlehurst
Copy link
Member

Hi, I have merged this and added some stuff to fix CI errors. If i accidentally missed some changes let me know, it was a bit messy with all the refactoring.

MatthewMiddlehurst
MatthewMiddlehurst previously approved these changes Jun 18, 2025
View reviewed changes
Copy link
Member

@MatthewMiddlehurst MatthewMiddlehurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

One open question is if we should remove _fit_predict() as it is just misleading for a semi-supervised approach, or should the user be able to use/test it even if ROCKAD is not intended to be used unsupervised?

Probably better to talk in the AD meeting coming up.

@MatthewMiddlehurst MatthewMiddlehurst merged commit a163862 into main Jun 19, 2025
17 checks passed
@MatthewMiddlehurst MatthewMiddlehurst deleted the pm/time_point_ROCKAD_refactor branch June 19, 2025 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SebastianSchmidl SebastianSchmidl SebastianSchmidl approved these changes

@MatthewMiddlehurst MatthewMiddlehurst MatthewMiddlehurst approved these changes

@TonyBagnall TonyBagnall Awaiting requested review from TonyBagnall

Assignees
No one assigned
Labels
anomaly detection Anomaly detection package enhancement New feature, improvement request or other non-bug code enhancement refactor Restructuring without changing its external behavior
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pattplatt @MatthewMiddlehurst @SebastianSchmidl