Conversation
* Address review comments: SPARC tempdir, SQUIM loop, public PPG API, SB revision - SPARC decode/convert: create Audio from waveform tensor directly, no dangling filepath to deleted tempdir - SQUIM: move model.to(device) before loop (was inside loop) - PPG: rename _extract_ppg_segments → extract_ppg_segments (public), _to_frame_major_posteriorgram → to_frame_major_posteriorgram (public), export from __init__.py, update tutorials and tests - SpeechBrain SER: add revision to cache key, pass revision to hf_hub_download, use yaml.safe_load instead of string splitting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix SpeechBrain YAML parsing: use permissive loader for custom tags SpeechBrain hyperparams.yaml uses custom YAML tags (!new:, !ref, etc.) that yaml.safe_load cannot handle. Use a dedicated SafeLoader subclass with a multi-constructor that ignores unknown tags. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix test function names: add missing underscore after 'test' prefix 7 test functions were named testXxx instead of test_Xxx, making them invisible to pytest discovery. Now all 14 PPG/segment tests are properly discovered and pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors SpeechBrain model loading to use a permissive YAML loader, makes PPG utility functions public, and fixes an issue in SPARC feature extraction where Audio objects relied on temporary files. It also optimizes model device placement in Torchaudio-Squim, though feedback suggests this may cause race conditions by modifying a global model instance.
| raise ValueError("Only 16000 Hz sampling rate is supported by Torchaudio-Squim model.") | ||
|
|
||
| features: List[Dict[str, Any]] = [] | ||
| model = _get_objective_model().to(device.value) |
There was a problem hiding this comment.
Moving the model to the device outside the loop is a good optimization. However, since _get_objective_model() returns a global model instance, calling .to(device.value) will modify the device of that global instance. In a multi-threaded environment where different threads might call this function with different devices, this could lead to race conditions and unexpected behavior as the model is moved back and forth between devices.
1 participant
SPARC tempdir fix, SQUIM loop opt, public PPG API, SpeechBrain YAML parsing, test name discovery fix.