Skip to content

Conversation

@shunping
Copy link
Collaborator

@shunping shunping commented Mar 10, 2025

  • Add transforms for anomaly detection

Depends on #34232

@github-actions
Copy link
Contributor

Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment assign set of reviewers

@shunping
Copy link
Collaborator Author

r: @damccorm

@github-actions
Copy link
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@shunping shunping force-pushed the anomaly-detection-4-2 branch 2 times, most recently from 3d1931e to 2642c70 Compare March 10, 2025 19:38
@shunping shunping force-pushed the anomaly-detection-4-2 branch from 2642c70 to d5aa691 Compare March 11, 2025 00:51
@shunping shunping self-assigned this Mar 11, 2025
@shunping shunping requested a review from damccorm March 11, 2025 01:11
@shunping shunping added this to the 2.64.0 Release milestone Mar 11, 2025
@codecov
Copy link

codecov bot commented Mar 11, 2025

Codecov Report

Attention: Patch coverage is 95.68966% with 5 lines in your changes missing coverage. Please review.

Project coverage is 56.35%. Comparing base (dfc45f0) to head (2ee0393).
Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
sdks/python/apache_beam/ml/anomaly/transforms.py 95.68% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #34234      +/-   ##
============================================
+ Coverage     56.28%   56.35%   +0.06%     
  Complexity     3286     3286              
============================================
  Files          1166     1173       +7     
  Lines        178678   179057     +379     
  Branches       3398     3398              
============================================
+ Hits         100575   100910     +335     
- Misses        74850    74894      +44     
  Partials       3253     3253              
Flag Coverage Δ
python 81.32% <95.68%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

model_id=self._underlying._model_id,
score=self.score_and_learn(data))]))

model_state.write(self._underlying)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a suggestion for this PR, but this would likely benefit from batching so that we don't have to do state reads/writes for every element (we could just do one per batch)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, though I think in the implementation states will be cached and they are not committed to the backend per element.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true - IIRC they are written when we commit, though (when we finish a bundle), which in streaming could be relatively frequently - a group into batches across bundles might help.

Regardless, that is something we can try/tune down the line and not something which needs to come right now

@shunping shunping force-pushed the anomaly-detection-4-2 branch from 99ed669 to 2ee0393 Compare March 11, 2025 17:54
@shunping
Copy link
Collaborator Author

Run Python_Coverage PreCommit

@shunping
Copy link
Collaborator Author

Run Prism_Python PreCommit 3.12

Copy link
Contributor

@damccorm damccorm left a comment

Choose a reason for hiding this comment

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

Thanks

@damccorm
Copy link
Contributor

I'll merge once checks pass

@damccorm damccorm merged commit a1b6a2a into apache:master Mar 11, 2025
94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants