Add support for remote zoo model parameters #5439
Conversation
WalkthroughThis pull request introduces enhancements to the FiftyOne model zoo documentation and utilities, focusing on improving zero-shot prediction capabilities and remote model handling. The changes span multiple files, including script modifications for model documentation generation, documentation updates for remote datasets and models, and updates to utility classes and methods for handling model parameters and loading. Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Poem
✨ Finishing Touches
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
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
fiftyone/zoo/models/__init__.py (3)
305-305: Ensure proper fallback when loading remote models
This newly introduced logic callsmodel._load_model(**kwargs)when no config is provided forRemoteZooModel. If_load_modelfails for any reason, there's currently no fallback. Consider wrapping the call in a try-except block or logging an informational message to aid in debugging.Also applies to: 307-307
505-519: Add docstrings for new helper methods
These new methods (_get_model_path,_load_model,_get_parameters,_parse_parameters) are crucial entry points for remote model handling, but they lack docstrings describing expected behavior and return values. Adding docstrings will help clarify usage and maintainability.
554-569: Consider logging whenget_parameters()/parse_parameters()are missing
Here,_get_remote_model_parametersand_parse_remote_model_parametersgracefully do nothing whenmoduledoesn’t define the relevant methods. Logging a brief message in such cases could improve discoverability and debugging.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/scripts/make_model_zoo_docs.py(1 hunks)docs/source/dataset_zoo/remote.rst(1 hunks)docs/source/model_zoo/remote.rst(3 hunks)fiftyone/utils/ultralytics.py(1 hunks)fiftyone/zoo/models/__init__.py(3 hunks)
✅ Files skipped from review due to trivial changes (2)
- fiftyone/utils/ultralytics.py
- docs/source/dataset_zoo/remote.rst
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.10)
- GitHub Check: test / test-app
- GitHub Check: lint / eslint
- GitHub Check: e2e / test-e2e
- GitHub Check: build / build
- GitHub Check: build
🔇 Additional comments (10)
docs/scripts/make_model_zoo_docs.py (2)
204-206: Nicely structured callback
Callingsession.refresh()after applying the model ensures that the session accurately reflects the newly generated predictions. This looks good.
207-215: Clear demonstration of zero-shot usage
This example snippet for zero-shot predictions with YOLO is straightforward and helpful for users.docs/source/model_zoo/remote.rst (8)
17-23: Useful references to external repos
Providing these repository examples is helpful for developers exploring remote model sources.
219-224: Well-defined function signatures
Declaringget_parameters()andparse_parameters()here clarifies the methods available for remote models.
431-432: Handy anchor for referencing parameter collection
This anchor makes documentation navigation more convenient for readers.
433-466: Comprehensive documentation for collecting model parameters
The new section provides a clear explanation of how to gather user-defined classes for zero-shot tasks.
469-471: Consistent cross-reference to operator inputs
Linking tooperator-inputsensures that readers know where to find more detailed input-collection references.
472-473: Concise anchor for parse-parameters
Continues the pattern of well-organized documentation anchors.
474-499: Useful example of parameter formatting
Demonstrates how remote models can adapt user-provided classes at execution time.
501-502: Consistent note referencing operator usage
This note helps users integrate custom parameter functionality into their existing operator workflows.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
fiftyone/operators/evaluation_metric.py (1)
48-58: LGTM: Method signature change improves API clarity.The replacement of
get_parameterswithresolve_inputsimplifies the interface while maintaining functionality. The new method has a clearer signature and explicit return type documentation.Consider adding examples in the docstring to demonstrate common usage patterns of the
resolve_inputmethod.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/source/model_zoo/remote.rst(3 hunks)docs/source/user_guide/evaluation.rst(2 hunks)fiftyone/operators/evaluation_metric.py(1 hunks)fiftyone/zoo/models/__init__.py(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: test / test-python (ubuntu-latest-m, 3.10)
- GitHub Check: test / test-app
- GitHub Check: lint / eslint
- GitHub Check: build / build
- GitHub Check: e2e / test-e2e
- GitHub Check: build
🔇 Additional comments (9)
docs/source/model_zoo/remote.rst (4)
17-22: LGTM: Clear example references added.The addition of example repository references (
voxel51/openai-clipandvoxel51/ultralytics-models) provides valuable guidance for users.
219-224: LGTM: Well-structured documentation for new methods.The introduction of
resolve_inputandparse_parametersmethods is clearly documented with proper indentation and formatting.
431-472: LGTM: Comprehensive documentation forresolve_input.The section provides clear explanation of the method's purpose, signature, and usage. The code example effectively demonstrates parameter collection.
478-508: LGTM: Clear documentation forparse_parameters.The section effectively explains the purpose and implementation of parameter parsing functionality.
fiftyone/zoo/models/__init__.py (4)
505-511: LGTM: Well-structured model path handling.The addition of
_get_model_pathandload_modelmethods improves code organization and reusability.
512-519: LGTM: Clean implementation of parameter handling methods.The
resolve_inputandparse_parametersmethods are well-integrated with the existing codebase and follow consistent error handling patterns.
554-572: LGTM: Robust helper function implementation.The helper functions
_resolve_remote_inputand_parse_remote_model_parametersprovide proper error handling and module validation.
305-305: Verify model loading path handling.The model loading logic has been updated to use instance methods. Ensure that model paths are correctly resolved in all scenarios.
Also applies to: 307-307
✅ Verification successful
Model path handling implementation verified
The model loading sequence properly resolves paths using
get_path_in_dirbefore loading the model, consistent with the established pattern throughout the codebase. The path resolution is correctly integrated with the model loading workflow.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify model path resolution rg -p "model_path" . --type py | grep -C 2 "get_path_in_dir"Length of output: 797
Script:
#!/bin/bash # Check get_path_in_dir implementation and model loading sequence ast-grep --pattern 'def get_path_in_dir($_) { $$$ }'Length of output: 56
Script:
#!/bin/bash # Check get_path_in_dir implementation with Python syntax ast-grep --pattern 'def get_path_in_dir($$$): $$$' # Also search with ripgrep for broader context rg -p "class.*get_path_in_dir" -A 5 --type pyLength of output: 104
docs/source/user_guide/evaluation.rst (1)
Line range hint
2114-2160: LGTM: Clear example of custom metric implementation.The example effectively demonstrates how to implement a custom evaluation metric with proper configuration and input handling.
Change log
resolve_input()andparse_parameters()methods to the remote zoo model interface. These methods allow remote zoo models to inject custom parameters into an operator's input form.See voxel51/fiftyone-plugins#201 for example usage.
TODO