packaging:drop dependency on neon package #8
Conversation
|
Warning Rate limit exceeded@JarbasAl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 30 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe changes in this pull request primarily involve the introduction of a new Changes
Possibly related PRs
Suggested labels
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 your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
ovos_stt_plugin_citrinet/engine.py (1)
127-135: Usetorch.no_grad()context during inference for efficiency.Wrapping inference code with
torch.no_grad()prevents unnecessary gradient calculations, improving performance and reducing memory usage.Modify the
_run_preprocessormethod:def _run_preprocessor(self, audio_16k: np.array): with torch.no_grad(): input_signal = torch.tensor(audio_16k).unsqueeze(0) length = torch.tensor(len(audio_16k)).unsqueeze(0) processed_signal, processed_signal_len = self.preprocessor.forward( input_signal=input_signal, length=length ) processed_signal = processed_signal.numpy() processed_signal_len = processed_signal_len.numpy() return processed_signal, processed_signal_lenConsider applying
torch.no_grad()in other methods where inference is performed.ovos_stt_plugin_citrinet/__init__.py (2)
Line range hint
29-29: Correct the typo in the docstring of theexecutemethod.The word "speach" should be corrected to "speech" in the docstring.
Apply this correction:
''' - Executes speach recognition + Executes speech recognition
Line range hint
54-68: Move example code to a separate script or test file.Including executable code under
__main__in__init__.pyis not recommended, as it can cause unintended side effects when the module is imported. Move this example code to a dedicated example script or test module.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ovos_stt_plugin_citrinet/__init__.py(1 hunks)ovos_stt_plugin_citrinet/engine.py(1 hunks)requirements.txt(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- requirements.txt
🔇 Additional comments (2)
ovos_stt_plugin_citrinet/__init__.py (2)
8-8: Ensure import statement reflects new module structure.
The import statement now sources Model and available_languages from ovos_stt_plugin_citrinet.engine. This aligns with the introduction of the new engine.py file.
11-12: Verify the default language setting in __init__.
The default language is set to "ca". Confirm that this is intentional and matches user expectations. If a different default language is preferred, consider updating it.
| def _trim_memory(): | ||
| """ | ||
| If possible, gives memory allocated by PyTorch back to the system | ||
| """ | ||
| libc = ctypes.CDLL("libc.so.6") | ||
| libc.malloc_trim(0) | ||
| gc.collect() | ||
|
|
There was a problem hiding this comment.
Ensure cross-platform compatibility in _trim_memory method.
The _trim_memory method uses libc.so.6, which is specific to Linux systems. This will raise an exception on other platforms like Windows or macOS. Consider adding a platform check or using a cross-platform approach for memory trimming.
Modify the method to check the operating system:
import platform
@staticmethod
def _trim_memory():
"""
If possible, gives memory allocated by PyTorch back to the system.
"""
if platform.system() == 'Linux':
libc = ctypes.CDLL("libc.so.6")
libc.malloc_trim(0)
gc.collect()| def _to_float32(self, audio_buffer: np.array): | ||
| audio_fp32 = np.divide(audio_buffer, np.iinfo(audio_buffer.dtype).max, dtype=np.float32) | ||
| return audio_fp32 |
There was a problem hiding this comment.
Handle floating-point data types in _to_float32 method.
The _to_float32 method assumes that audio_buffer has an integer data type. If audio_buffer is already a floating-point array, np.iinfo will raise an error. Add a check to handle floating-point inputs appropriately.
Update the method to accommodate different data types:
def _to_float32(self, audio_buffer: np.array):
if np.issubdtype(audio_buffer.dtype, np.integer):
max_val = np.iinfo(audio_buffer.dtype).max
audio_fp32 = np.divide(audio_buffer, max_val, dtype=np.float32)
elif np.issubdtype(audio_buffer.dtype, np.floating):
audio_fp32 = audio_buffer.astype(np.float32)
else:
raise ValueError(f"Unsupported audio buffer data type: {audio_buffer.dtype}")
return audio_fp32
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
README.md (1)
12-17: Improve formatting and punctuation of installation instructions.The installation instructions could be clearer with better formatting and punctuation:
-by default this plugin will install the full pytorch, to avoid dragging all the dependencies it is recommended you install the cpu only version of pytorch **before** installing the plugin +By default, this plugin will install the full PyTorch. To avoid dragging all the dependencies, it is recommended you install the CPU-only version of PyTorch **before** installing the plugin. -`pip install torch==2.1.0+cpu -f https://download.pytorch.org/whl/torch_stable.html` +```bash +pip install torch==2.1.0+cpu -f https://download.pytorch.org/whl/torch_stable.html +``` -If you skip the step above then the full pytorch will be installed together with the plugin +If you skip the step above, then the full PyTorch will be installed together with the plugin.🧰 Tools
🪛 LanguageTool
[uncategorized] ~12-~12: A comma might be missing here.
Context: ...e used with this plugin ## Install by default this plugin will install the full pytor...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~12-~12: A period might be missing here.
Context: ...on of pytorch before installing the plugin `pip install torch==2.1.0+cpu -f http...(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[typographical] ~16-~16: Consider adding a comma.
Context: ...stable.html` If you skip the step above then the full pytorch will be installed toge...(IF_THEN_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.md(1 hunks)requirements.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- requirements.txt
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~12-~12: A comma might be missing here.
Context: ...e used with this plugin ## Install by default this plugin will install the full pytor...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~12-~12: A period might be missing here.
Context: ...on of pytorch before installing the plugin `pip install torch==2.1.0+cpu -f http...
(AI_EN_LECTOR_MISSING_PUNCTUATION_PERIOD)
[typographical] ~16-~16: Consider adding a comma.
Context: ...stable.html` If you skip the step above then the full pytorch will be installed toge...
(IF_THEN_COMMA)
🔇 Additional comments (1)
README.md (1)
14-14: Verify PyTorch version compatibility.
Let's verify if the specified PyTorch version (2.1.0) matches the version constraints in requirements.txt.
✅ Verification successful
PyTorch version in README.md is compatible with requirements.txt
The PyTorch version (2.1.0) specified in the installation command matches the version constraint in requirements.txt (torch>=2.1.0), ensuring compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check PyTorch version specification in requirements.txt
rg "torch[>=<]" requirements.txt
Length of output: 47
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
ovos_stt_plugin_citrinet/engine.py (4)
97-106: Enhance error messages in_init_model_from_pathmethod.The method raises a
ValueErrorif the provided path is not valid, but additional checks on the existence of required files could improve error reporting.Consider updating the method to check for the required files:
def _init_model_from_path(self, path: str): if not os.path.isdir(path): raise ValueError(f"'{path}' is not a valid model directory.") preprocessor_path = os.path.join(path, "preprocessor.ts") encoder_path = os.path.join(path, "model.onnx") tokenizer_path = os.path.join(path, "tokenizer.spm") if not all(os.path.isfile(p) for p in [preprocessor_path, encoder_path, tokenizer_path]): raise ValueError(f"Model files are missing in '{path}'. Expected 'preprocessor.ts', 'model.onnx', and 'tokenizer.spm'.") self._init_preprocessor(preprocessor_path) self._init_encoder(encoder_path) self._init_tokenizer(tokenizer_path) self._trim_memory()
172-173: Optimize memory trimming insttmethod.Calling
_trim_memoryafter each STT operation might introduce performance overhead. Assess whether it's necessary to trim memory every time or if it can be called less frequently.
175-179: Add error handling tostt_filemethod.The
stt_filemethod does not handle exceptions that may occur during file reading or processing. Consider adding try-except blocks to provide meaningful error messages to the users.def stt_file(self, file_path: str): try: audio_buffer, sr = self.read_file(file_path) current_hypotheses = self.stt(audio_buffer, sr) return current_hypotheses except Exception as e: raise RuntimeError(f"An error occurred while processing the file '{file_path}': {e}")
197-199: Handle exceptions in_resamplemethod.The
soxr.resamplefunction may raise exceptions if the input parameters are invalid. Adding error handling can improve the robustness of the method.Consider wrapping the resampling in a try-except block:
def _resample(self, audio_fp32: np.array, sr: int): try: audio_16k = soxr.resample(audio_fp32, sr, self.sample_rate) return audio_16k except Exception as e: raise RuntimeError(f"Resampling failed: {e}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ovos_stt_plugin_citrinet/engine.py(1 hunks)
🔇 Additional comments (5)
ovos_stt_plugin_citrinet/engine.py (5)
1-30: Review licensing and attribution in the header comments.
The file contains licensing and contributor information from the NeonGecko project. Ensure that the licensing terms are compatible with this project, and update the attribution if necessary to reflect the contributors and license applicable to your codebase.
88-90: Good job adding error handling for unsupported languages in _init_model.
The inclusion of a check for unsupported languages improves the robustness of the model initialization process.
130-137: Verify the input shapes and types in _run_preprocessor method.
Ensure that the input tensors are correctly shaped and of the expected data type to prevent runtime errors during the preprocessing step.
189-196:
Ensure cross-platform compatibility in _trim_memory method.
The _trim_memory method uses libc.so.6, which is specific to Linux systems. This will raise an exception on other platforms like Windows or macOS. Consider adding a platform check or using a cross-platform approach for memory trimming.
Apply this change to handle cross-platform compatibility:
import platform
@staticmethod
def _trim_memory():
"""
If possible, gives memory allocated by PyTorch back to the system.
"""
if platform.system() == 'Linux':
libc = ctypes.CDLL("libc.so.6")
libc.malloc_trim(0)
gc.collect()201-203:
Handle floating-point data types in _to_float32 method.
The _to_float32 method assumes that audio_buffer has an integer data type. If audio_buffer is already a floating-point array, np.iinfo will raise an error. Add a check to handle floating-point inputs appropriately.
Update the method to accommodate different data types:
def _to_float32(self, audio_buffer: np.array):
if np.issubdtype(audio_buffer.dtype, np.integer):
max_val = np.iinfo(audio_buffer.dtype).max
audio_fp32 = np.divide(audio_buffer, max_val, dtype=np.float32)
elif np.issubdtype(audio_buffer.dtype, np.floating):
audio_fp32 = audio_buffer.astype(np.float32)
else:
raise ValueError(f"Unsupported audio buffer data type: {audio_buffer.dtype}")
return audio_fp32
Summary by CodeRabbit
New Features
Modelclass for enhanced speech-to-text functionality.Bug Fixes
Chores
requirements.txtto include necessary libraries for STT functionality and audio processing.streaming-stt-nemo.