-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat/reranker+optional_ovos_classifiers #529
Conversation
ovos-classifiers is still in pre-alpha and wont have a stable release anytime soon, make it an optional dependency of ovos-core 0.0.8
WalkthroughThe recent updates enhance the stability and efficiency of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CommonQAService
participant Reranker
User->>CommonQAService: Ask a question
CommonQAService->>CommonQAService: Check for Reranker
alt Reranker available
CommonQAService->>Reranker: Use to select answer
else Reranker not available
CommonQAService->>CommonQAService: Randomly select first answer
end
CommonQAService->>User: Provide answer
sequenceDiagram
participant User
participant OCPService
participant Classifier
User->>OCPService: Submit media
OCPService->>OCPService: Check if Classifier is enabled
alt Classifier enabled
OCPService->>Classifier: Classify media
else Classifier not enabled
OCPService->>OCPService: Skip classification
end
OCPService->>User: Provide results
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- ovos_core/intent_services/commonqa_service.py (3 hunks)
- ovos_core/intent_services/ocp_service.py (7 hunks)
- requirements/requirements.txt (1 hunks)
Files skipped from review due to trivial changes (1)
- requirements/requirements.txt
Additional comments not posted (8)
ovos_core/intent_services/commonqa_service.py (2)
46-55
: Improved error handling for re-ranking component initialization.The try-except block ensures that the service remains functional even if the
BM25MultipleChoiceSolver
import fails. This improves the robustness of the service.
264-270
: Enhanced decision-making process for selecting the best answer.The changes ensure that the service can still function even when the re-ranking component is unavailable. This improves the robustness of the service.
ovos_core/intent_services/ocp_service.py (6)
Line range hint
152-157
:
Conditional import ofSklearnOVOSClassifier
for binary classifiers.The conditional import ensures that the classifier is only imported when necessary, potentially reducing overhead during initialization. This improves resource management and performance.
Line range hint
163-168
:
Conditional import ofSklearnOVOSClassifier
for media type classifiers.The conditional import ensures that the classifier is only imported when necessary, potentially reducing overhead during initialization. This improves resource management and performance.
Line range hint
732-745
:
Efficient media type classification using conditional import ofSklearnOVOSClassifier
.The conditional import and use of the classifier ensure that the media type is determined efficiently based on the query. This improves resource management and performance.
Line range hint
758-772
:
Efficient playback question classification using conditional import ofSklearnOVOSClassifier
.The conditional import and use of the classifier ensure that the playback question is determined efficiently based on the query. This improves resource management and performance.
1250-1254
: Lazy initialization ofKeywordFeaturesVectorizer
inOCPFeaturizer
.The lazy initialization strategy ensures that the vectorizer is only created when explicitly needed, optimizing resource usage.
1250-1254
: Lazy initialization ofKeywordFeaturesVectorizer
inOCPFeaturizer
.The lazy initialization strategy ensures that the vectorizer is only created when explicitly needed, optimizing resource usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- ovos_core/intent_services/ocp_service.py (10 hunks)
- requirements/tests.txt (1 hunks)
- test/unittests/skills/test_ocp.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- ovos_core/intent_services/ocp_service.py
Additional comments not posted (2)
requirements/tests.txt (1)
22-23
: LGTM! Verify compatibility with other dependencies.The addition of the
ovos-classifiers
package with the specified version constraint and the accompanying comment is appropriate.However, ensure that this new dependency does not conflict with existing ones.
Verification successful
No conflicts found with the addition of
ovos-classifiers
dependency.The
ovos-classifiers
package with the specified version constraint does not conflict with existing dependencies inrequirements/tests.txt
.
requirements/tests.txt
contains unique package names with no overlapping version constraints.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of `ovos-classifiers` with other dependencies. # Test: Check for conflicts. Expect: No conflicts. pip install -r requirements/tests.txtLength of output: 110
Script:
#!/bin/bash # Description: Check for version conflicts in requirements/tests.txt # List all packages with their version constraints in the requirements file cat requirements/tests.txt | grep -E '^[a-zA-Z0-9\-]+[<>=!~]' # Check for any overlapping or conflicting version constraints awk '/^[a-zA-Z0-9\-]+[<>=!~]/ {print $1}' requirements/tests.txt | sort | uniq -dLength of output: 507
test/unittests/skills/test_ocp.py (1)
21-21
: LGTM! Verify the new import path.The modification to the import path for the
load_entities
method is appropriate given the context.However, ensure that the new import path is correct and that the method functions as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- ovos_core/intent_services/ocp_service.py (13 hunks)
Files skipped from review as they are similar to previous changes (1)
- ovos_core/intent_services/ocp_service.py
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #529 +/- ##
==========================================
- Coverage 75.70% 75.37% -0.34%
==========================================
Files 15 15
Lines 3042 3086 +44
==========================================
+ Hits 2303 2326 +23
- Misses 739 760 +21
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
ovos_core/intent_services/commonqa_service.py (1)
34-34
: Constructor Update: Ensure all instantiations are updated.The constructor now accepts an optional
config
parameter. Ensure all instantiations ofCommonQAService
are updated to pass the new parameter if needed. Specifically, update the instantiation intest/unittests/common_query/test_common_query.py
to include theconfig
parameter if configuration testing is required.
test/unittests/common_query/test_common_query.py
: Update instantiation to includeconfig
.Analysis chain
Constructor Update: Ensure all instantiations are updated.
The constructor now accepts an optional
config
parameter. Ensure all instantiations ofCommonQAService
are updated to pass the new parameter if needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `CommonQAService` are updated to pass the new `config` parameter if needed. # Test: Search for the class instantiation. Expect: Occurrences of the new signature. rg --type python -A 5 $'CommonQAService('Length of output: 125
Script:
#!/bin/bash # Description: Verify all instantiations of `CommonQAService` are updated to pass the new `config` parameter if needed. # Test: Search for the class instantiation. Expect: Occurrences of the new signature. rg --type python -A 5 $'CommonQAService\\('Length of output: 74
Script:
#!/bin/bash # Description: Verify all instantiations of `CommonQAService` are updated to pass the new `config` parameter if needed. # Test: Search for the class instantiation. Expect: Occurrences of the new signature. rg -t py -A 5 $'CommonQAService\\('Length of output: 1652
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- ovos_core/intent_services/init.py (1 hunks)
- ovos_core/intent_services/commonqa_service.py (3 hunks)
Additional comments not posted (2)
ovos_core/intent_services/commonqa_service.py (1)
263-269
: Robustness Improvement: Default behavior for missing re-ranker.The changes improve robustness by ensuring the service remains functional even when the re-ranker is unavailable. The default behavior of picking the first answer is a reasonable fallback.
ovos_core/intent_services/__init__.py (1)
67-67
: Configuration Update: Ensure consistency withCommonQAService
.The change ensures that the
CommonQAService
is instantiated with the appropriate configuration, aligning with the changes made incommonqa_service.py
.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- ovos_core/intent_services/commonqa_service.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- ovos_core/intent_services/commonqa_service.py
closes #436 |
``` 2024-08-04 01:46:56.692 - skills - ovos_core.intent_services.commonqa_service:_query_timeout:269 - DEBUG - Tied skills: ['skill-ovos-ddg.openvoiceos', 'skill-ovos-wikipedia.openvoiceos', 'skill-ovos-wolfie.openvoiceos'] 2024-08-04 01:46:56.693 - skills - ovos_core.intent_services.commonqa_service:_query_timeout:275 - DEBUG - ReRanking answers from skills 2024-08-04 01:46:56.974 - skills - ovos_core.intent_services.commonqa_service:_query_timeout:280 - INFO - ReRanked score: 0.9994009733200073 - {'phrase': 'what is the speed of light', 'skill_id': 'skill-ovos-ddg.openvoiceos', 'answer': 'The speed of light in vacuum, commonly denoted c, is a universal physical constant that is exactly equal to 299,792,458 metres per second.', 'handles_speech': True, 'callback_data': {'query': 'what is the speed of light', 'answer': 'The speed of light in vacuum, commonly denoted c, is a universal physical constant that is exactly equal to 299,792,458 metres per second.'}, 'conf': 2.229565217391304} 2024-08-04 01:46:56.975 - skills - ovos_core.intent_services.commonqa_service:_query_timeout:280 - INFO - ReRanked score: 0.9994009733200073 - {'phrase': 'what is the speed of light', 'skill_id': 'skill-ovos-wikipedia.openvoiceos', 'answer': 'The speed of light in vacuum, commonly denoted c, is a universal physical constant that is exactly equal to 299,792,458 metres per second .', 'handles_speech': True, 'callback_data': {'query': 'what is the speed of light', 'image': 'https://upload.wikimedia.org/wikipedia/commons/2/2e/Earth_to_Sun_-_en.png', 'title': 'Speed Of Light', 'answer': 'The speed of light in vacuum, commonly denoted c, is a universal physical constant that is exactly equal to 299,792,458 metres per second .'}, 'conf': 2.1133333333333333} 2024-08-04 01:46:56.975 - skills - ovos_core.intent_services.commonqa_service:_query_timeout:280 - INFO - ReRanked score: 0.9967312812805176 - {'phrase': 'what is the speed of light', 'skill_id': 'skill-ovos-wolfie.openvoiceos', 'answer': 'The speed of light has a value of about 300 million meters per second', 'handles_speech': True, 'callback_data': {'query': 'what is the speed of light', 'answer': 'The speed of light has a value of about 300 million meters per second'}, 'conf': 2.8085714285714287} 2024-08-04 01:46:56.975 - skills - ovos_core.intent_services.commonqa_service:_query_timeout:287 - INFO - Handling with: skill-ovos-ddg.openvoiceos ``` add "ignore_skill_scores" config option, rely on the reranker for best answer selection while ignore self reported confidence by skills
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
ovos_core/intent_services/commonqa_service.py (1)
35-59
: Constructor changes approved.The changes enhance flexibility and robustness by allowing dynamic configuration and handling potential plugin loading failures. However, the instantiation in the test file should be updated to pass the
config
parameter.
test/unittests/common_query/test_common_query.py
: Update the instantiation to include theconfig
parameter.Analysis chain
Constructor changes approved.
The changes enhance flexibility and robustness by allowing dynamic configuration and handling potential plugin loading failures.
However, ensure that all instantiations of
CommonQAService
pass the correct configuration.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all instantiations of `CommonQAService` pass the correct configuration. # Test: Search for the class instantiation. Expect: Occurrences of the new constructor signature. rg --type python -A 5 $'CommonQAService('Length of output: 125
Script:
#!/bin/bash # Description: Verify all instantiations of `CommonQAService` pass the correct configuration. # Test: Search for the class instantiation. Expect: Occurrences of the new constructor signature. rg --type python -A 5 'CommonQAService\('Length of output: 74
Script:
#!/bin/bash # Description: Verify all instantiations of `CommonQAService` pass the correct configuration. # Test: Search for the class instantiation. Expect: Occurrences of the new constructor signature. rg --type py -A 5 'CommonQAService\('Length of output: 1656
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- ovos_core/intent_services/commonqa_service.py (3 hunks)
Additional comments not posted (1)
ovos_core/intent_services/commonqa_service.py (1)
257-284
: Answer selection logic changes approved.The changes improve the answer selection process by utilizing the re-ranking functionality if available.
ovos-classifiers is still in pre-alpha and wont have a stable release anytime soon, make it an optional dependency of ovos-core 0.0.8
related #488
solves TODO to allow reranker to come from config instead of being hardcoded, closes #436
improves logs about reranking process for clarity
a new config option (default False) allows using the reranker while ignoring the skill self reported confidences
in example below wolfram alpha would normally be selected, but the reranker preferred the duck duck go answer
recommend plugin: https://github.com/TigreGotico/ovos-flashrank-reranker-plugin
Summary by CodeRabbit
New Features
Bug Fixes
Chores
ovos-classifiers
, simplifying project requirements.ovos-classifiers
dependency to the testing requirements for enhanced testing capabilities.