fix: relativeOdi denominators dilute on contact-loss dropouts - #68
Conversation
…not raw span same bug shape as the IS fix (#67). a contact-loss dropout (arm tucked, band shifted) turns part of the ratio-of-ratios NaN, but odiPerHour and burdenPct still divided by the full wall-clock span, diluting the rate instead of reporting it over the time that actually had usable samples. now scales the denominator by trustedCoverage, same idea cvhr_apnea.dart already uses.
There was a problem hiding this comment.
Sorry @abdulsaheel, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 6 days and 7 hours by commenting @sourcery-ai review. Upgrade to get a review now.
|
Warning Review limit reachedNext included review available in 38 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideCorrects relative ODI rate and burden calculations so unusable ratio-of-ratios samples from contact-loss dropouts do not dilute results; the change is isolated to metric computation and is not yet wired into edge callers. Flow diagram for trusted relative ODI denominator calculationflowchart TD
A[relR ratio-of-ratios samples] --> B{Sample is usable}
B -->|yes| C[trustedCoverage]
B -->|no: NaN| C
C --> D[validSpanSec = spanSec * trustedCoverage]
D --> E[validHours = validSpanSec / 3600]
E --> F[odiPerHour = dipCount / validHours]
D --> G[burdenPct = totalDipSec / validSpanSec]
C --> H[trustedCoverage reported unchanged]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
…nflict combined #68's per-sample trustedCoverage scaling with #69's gap segmentation: each segment now computes its own trustedCoverage and scales its own hours contribution before summing into analyzedHours, instead of one global scale applied after the fact. odiPerHour and burdenPct both key off that combined analyzedHours.
odiPerHour and burdenPct in relativeOdi() were dividing by the raw wall-clock span, not by time that actually produced a usable ratio-of-ratios sample. dcRed==0/dcIr==0/rIr<=0 turn part of relR into NaN on a contact-loss dropout (arm tucked, band shifted), and that gap was still charged to analyzedHours/spanSec, so the rate gets diluted downward instead of reported over the monitored time.
same shape as the IS fix in #67, and cvhr_apnea.dart in this same family already builds analyzedHours only from segments with usable data for exactly this reason.
fix scales analyzedHours/spanSec by trustedCoverage (the already-computed fraction of non-NaN ratio samples) before the odiPerHour/burdenPct divisions. trustedCoverage itself is unchanged, still reported as-is.
not wired into edge yet so no caller-visible behavior change, just fixes the metric before anything starts consuming it.
Summary by Sourcery
Calculate relative ODI rates and burden over trusted monitored time rather than the full wall-clock span.
Bug Fixes:
Enhancements: