-
Notifications
You must be signed in to change notification settings - Fork 0
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
Small fix to find_global_peaks_rough #28
Conversation
WalkthroughThe recent changes focus on enhancing the peak detection process in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on X ? TipsChat with CodeRabbit Bot (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #28 +/- ##
=======================================
Coverage 99.78% 99.78%
=======================================
Files 19 19
Lines 949 952 +3
=======================================
+ Hits 947 950 +3
Misses 2 2 ☔ View full report in Codecov by Sentry. |
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sleap_nn/inference/peak_finding.py (1 hunks)
Additional comments: 1
sleap_nn/inference/peak_finding.py (1)
- 118-119: The code correctly replaces peak values below the threshold with NaN and sets the corresponding
max_values
to 0. This change aligns with the PR objectives and summary, which mention adjusting the handling of values below a specified threshold.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sleap_nn/inference/peak_finding.py (1 hunks)
Additional comments: 2
sleap_nn/inference/peak_finding.py (2)
118-121: The change from replacing values below the threshold with
NaN
to0
could have implications on downstream processing that relies onNaN
to indicate invalid or absent data. Ensure that this change is communicated to all users of the function and that it does not negatively impact any dependent code.113-115: The summary does not mention the change in the order of concatenation for peak indices (
max_indices_x
andamax_indices_y
). This change in the order of peak points could affect how the peaks are interpreted downstream. It is important to verify that this change is intentional and that it does not introduce any issues with the interpretation of peak data.
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.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (1)
- sleap_nn/inference/peak_finding.py (1 hunks)
Additional comments: 1
sleap_nn/inference/peak_finding.py (1)
- 113-115: The
peak_points
tensor is constructed by concatenatingmax_indices_x
andamax_indices_y
, which are derived from separate maximum value calculations. This mixing of indices from different calculations is unusual and could be intentional for a specific reason, but it should be clarified to ensure it's not a mistake.
Summary by CodeRabbit