[SequenceFeatureExtractor] Rewrite padding logic from pure python to numpy #13650
Conversation
anton-l
requested review from
patrickvonplaten,
patil-suraj,
LysandreJik and
sgugger
| x = np.subtract(x, mean) | ||
| if normalize_vars: | ||
| var = square_sums / x[:input_length].shape[0] - mean ** 2 | ||
| std = np.sqrt(np.maximum(var, 1e-10)) | ||
| std = x[:input_length].std(axis=0) |
There was a problem hiding this comment.
Switched this logic to pure numpy to squeeze out a bit more precision when working with np.float32 instead of casted float->np.float64
| if is_batched and not isinstance(raw_speech[0], np.ndarray): | ||
| raw_speech = [np.asarray(speech) for speech in raw_speech] | ||
| if is_batched: | ||
| raw_speech = [np.asarray(speech, dtype=np.float32) for speech in raw_speech] |
There was a problem hiding this comment.
Forcing float32 here for consistency with other feature extractors
anton-l
changed the title
anton-l
changed the title
| @@ -724,7 +724,7 @@ def map_to_array(batch): | |||
| return batch | |||
|
|
|||
| ds = load_dataset("patrickvonplaten/librispeech_asr_dummy", "clean", split="validation") | |||
| ds = ds.select(range(num_samples)).map(map_to_array) | |||
| ds = ds.sort("id").select(range(num_samples)).map(map_to_array) | |||
There was a problem hiding this comment.
Unrelated to this PR: the newdatasets version re-shuffled this dataset, so sorting is needed for reproducibility.
| output = speech_recognizer(waveform) | ||
| self.assertEqual(output, {"text": ""}) | ||
|
|
||
| from datasets import load_dataset | ||
|
|
||
| ds = load_dataset("patrickvonplaten/librispeech_asr_dummy", "clean", split="validation") | ||
| filename = ds[0]["file"] | ||
| ds = load_dataset("patrickvonplaten/librispeech_asr_dummy", "clean", split="validation").sort("id") |
There was a problem hiding this comment.
Unrelated to this PR: the newdatasets version re-shuffled this dataset, so sorting is needed for reproducibility.
| @@ -42,9 +42,9 @@ def test_torch_small(self): | |||
| tokenizer="facebook/s2t-small-mustc-en-fr-st", | |||
| framework="pt", | |||
| ) | |||
| waveform = np.zeros((34000,)) | |||
| waveform = np.linspace(0, 1, 34000, dtype=np.float32) | |||
There was a problem hiding this comment.
Now that the slight variability due to type-casting is eliminated, this needs to be something specific, because the model becomes unstable (non-reproducible in different environments) with all-zero inputs.
|
Also did you notice a speed-up for larger inputs? |
|
@patrickvonplaten the benchmarking results are pretty promising:
|
|
Great job @anton-l - feel free to merge! |
…numpy (huggingface#13650) * Test np padding * Pass feature extraction tests * Update type hints * Fix flaky integration tests * Try a more stable waveform * Add to_numpy jax support * int32 attention masks * Refactor normalization tests
…numpy (huggingface#13650) * Test np padding * Pass feature extraction tests * Update type hints * Fix flaky integration tests * Try a more stable waveform * Add to_numpy jax support * int32 attention masks * Refactor normalization tests
What does this PR do?
Resolves #13539
Since speech models universally use Numpy
float32arrays as input features (standard way of representing waveforms), it was decided to rewriteSequenceFeatureExtractorfrom pure python lists (akin to traditional tokenizers) to numpy arrays. It will also help with solving some inconsistent normalization issues (#13538, #13585) due tofloat->np.float32conversions.The feature extractor itself is still dtype-agnostic (can pad
np.float64in the future if needed), while the model-specific feature extractors were updated to only work withnp.float32