[model-gateway] add phi3 vision image processor#14381
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the phi3-vision model, a valuable addition. The changes are well-structured, including a new Rust image processor, updates to core data structures to handle 5D tensors, and a Python script for generating test data. The implementation is comprehensive, with good documentation and tests.
My review focuses on improving performance and robustness. I've identified several areas where ndarray operations can be made more efficient by avoiding unnecessary data copies and allocations. Additionally, I've suggested a change to make accessor methods safer by handling unexpected tensor dimensions more gracefully. These refinements will enhance the code's performance and reliability.
| .map_err(|e| TransformError::InvalidShape { | ||
| expected: "valid 5D shape".to_string(), | ||
| actual: vec![e.to_string().len()], | ||
| })?; |
There was a problem hiding this comment.
The error message here is not very helpful for debugging. e.to_string().len() is used as the actual shape, which is incorrect. It would be more informative to include the shape that caused the failure and the underlying error message from ndarray.
.map_err(|e| TransformError::InvalidShape {
expected: format!("valid 5D shape, but failed with error: {}", e),
actual: shape,
})?
Checklist