Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the event rate calculation to improve performance by replacing the generator-based implementation with a composite processor architecture and introducing a new BinnedEventAggregator for more efficient event binning.
Key changes:
- Refactored
EventRatefrom generator-based to composite processor pattern using composition of smaller transformers - Added new
BinnedEventAggregatorwith optimized binning logic that handles overflow states - Created reusable
Aggregatetransformer for axis-based aggregation operations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ezmsg/event/rate.py | Replaced generator-based rate calculation with composite processor architecture using WindowTransformer, Aggregate, RenameAxis, and DensifyAndScale components |
| src/ezmsg/event/binned.py | Added new BinnedEventAggregator with stateful processing for efficient event binning with overflow handling |
| src/ezmsg/event/aggregate.py | Created new Aggregate transformer for performing sum/mean operations along specified axes |
| src/ezmsg/event/init.py | Updated exports to include new Rate, EventRate, Aggregate, and BinnedEventAggregator classes |
| tests/test_rate.py | Renamed test function, updated to use new API, added performance assertions, and created comprehensive test for BinnedEventAggregator |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It's been a few months since I last looked at this code and I can't remember what prompted it. I think the current event->rate calculation is slow and this is a faster version, but I can't recall with certainty.