Skip to content

Impulse Response Fatigue Model, EMA Smoothing, and Acoustic Feature Extraction#2

Merged
Khubaib8281 merged 13 commits into
mainfrom
feat/sarib-impulse-response
Jul 25, 2026
Merged

Impulse Response Fatigue Model, EMA Smoothing, and Acoustic Feature Extraction#2
Khubaib8281 merged 13 commits into
mainfrom
feat/sarib-impulse-response

Conversation

@SaribAzim

Copy link
Copy Markdown
Collaborator

Adds the Week 2 implementation for the design approved in the previous PR: exponential moving average smoothing for the raw fatigue score, the vocal load impulse response model with acute and chronic components and recovery time estimation, and acoustic feature extraction (jitter, shimmer, HNR, CPPS) wired into the existing processing pipeline. A minimal, backward compatible addition was made to session.py to support the new features field, full session level integration is planned for Week 3 as scoped in the design document.

@Khubaib8281
Khubaib8281 self-requested a review July 24, 2026 03:44

@Khubaib8281 Khubaib8281 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I appreciate the detailed implementation. The PR is directionally aligned with the planned Week 3 scope and introduces three useful components: EMA smoothing for stabilizing the raw fatigue signal, an impulse-response-based vocal load model with acute/chronic components and recovery estimation, and Praat/Parselmouth-based acoustic feature extraction for jitter, shimmer, HNR, and CPPS.

A few points I'd recommend addressing before merging:

  1. Clarify the status of acoustic features: The features are currently extracted and stored in the session records, but they do not yet appear to influence the fatigue score, warning logic, or downstream analytics. This is fine given the stated Week 3 scope, but it would be useful to explicitly document them as auxiliary features collected for future analysis rather than implying they are currently part of the fatigue estimation model.

  2. Review the readiness metric: The chronic - acute formulation is inspired by impulse-response models used in training-load research, but the interpretation of this value as vocal "readiness" is not yet validated for vocal fatigue. I would suggest either documenting this as an experimental metric or initially exposing acute/chronic load separately until the formulation is empirically validated.

  3. Timestamp consistency: SessionAnalytics.add() receives ts, but the impulse-response model currently uses time.monotonic() to calculate elapsed time instead. It would be good to clarify and standardize the time source so that the session timestamp and temporal load calculations remain consistent.

  4. Error visibility: Acoustic extraction intentionally fails gracefully, which is good for keeping monitoring uninterrupted. However, except Exception: return {} can make debugging difficult. Consider logging the extraction failure while still returning an empty feature dictionary.

  5. Tests and CI: Before merging, I recommend adding focused tests for EMA behavior, impulse-response decay/accumulation, recovery ETA, session summary outputs, and the acoustic extraction failure path. These tests should also be integrated into the project's CI pipeline so that every push and pull request automatically runs the test suite and verifies that existing functionality has not regressed. Please also include a test coverage report in CI, so we can track coverage for the newly added code and ensure that the critical analytics and processing paths are adequately tested. The CI checks should pass before the PR is considered ready to merge.

One general recommendaion for future contributions: where possible, please keep PRs small and focused around a single logical change rather than combining multiple Week-level tasks into one PR. For example, EMA smoothing, impulse-response load modeling, and acoustic feature extraction could each be introduced and reviewed independently, or grouped only when they are tightly coupled.

Overall, I think the additions are a good foundation for the planned functionality. With the above points clarified and covered, particularly the distinction between currently stored acoustic features and features actively used by the fatigue model, this should be in a good position to merge.

@SaribAzim

Copy link
Copy Markdown
Collaborator Author

Thanks for the detailed review. Addressed each point:

  1. Documented acoustic features as auxiliary, collected for future analysis, not yet feeding the fatigue score or warning logic (session.py, audio_stream.py).
  2. Renamed readiness to readiness_experimental and added a clear docstring noting it is unvalidated for vocal fatigue. Acute and chronic remain separately exposed.
  3. Removed the wall clock dependency entirely. Elapsed time for decay is now derived from actual audio samples processed (samples / sample rate), the same deterministic clock that produces session timestamps, so they cannot drift apart.
  4. Extraction failures and a missing parselmouth install now both log a warning instead of failing silently.
  5. Added 14 unit tests covering EMA behavior, impulse decay/accumulation, recovery ETA (checked against the manual formula), session summary shape, and the acoustic extraction failure path, plus a GitHub Actions workflow that runs the suite with coverage on every push and PR to main.

Also noted on keeping future PRs scoped to a single logical change, planning to split Week 3 work (baseline calibration, further session integration) into separate PRs rather than bundling them.

@Khubaib8281 Khubaib8281 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks for thoughtfully addressing the review comments.

Concern:
The coverage report shows 60% overall test coverage of code, with
utils.py | 57.1%
session.py | 46.2%
audio_stream.py | 33.8%
cli.py | 0%

kindly address this issue , however the coverage for analytics.py, init.py, config.py is very solid but the mentioned files need much improvement. Once the test coverage is improved, I'll be happy to merge changes.

@SaribAzim

Copy link
Copy Markdown
Collaborator Author

Added tests for utils.py, session.py, audio_stream.py, and cli.py, using mocking for the hardware dependent parts (microphone input, command line args) since those can't run in CI. Coverage is now at 93% overall, with utils.py and session.py at 100%.

@Khubaib8281 Khubaib8281 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

solid improvement in test coverage. PR is approved

@Khubaib8281
Khubaib8281 merged commit 255fb5f into main Jul 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants