Skip to content
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

Draft: Resnet support added #246

Merged
merged 9 commits into from
May 31, 2024
Merged

Draft: Resnet support added #246

merged 9 commits into from
May 31, 2024

Conversation

I8dNLo
Copy link
Contributor

@I8dNLo I8dNLo commented May 21, 2024

Added support of resnet-50, classical CNN

},
{
"model": "AndrewOgn/resnet_onnx",
"dim": 2048,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just as a reminder: we might want to inspect other resnet models to have lower dimensionality

fastembed/image/transform/operators.py Outdated Show resolved Hide resolved
Shapes matching for Resnet50-onnx
Example of Resnet50 to onnx conversion (basic)
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@I8dNLo I8dNLo requested a review from joein May 23, 2024 11:12
@@ -59,6 +59,9 @@ def __init__(self, scale: float = 1 / 255):
def __call__(self, images: List[np.ndarray]) -> List[np.ndarray]:
return [rescale(image, scale=self.scale) for image in images]

class PILtoNDarray:
def __call__(self, images: List[Union[Image.Image, np.ndarray]]) -> List[np.ndarray]:
return [np.asarray(image).swapaxes(2, 0) if isinstance(image, Image.Image) else image for image in images]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems to be

(H, W, C) -> (C, W, H)

but should be (H, W, C) -> (C, H, W)

so we need to use transpose((2, 0, 1)) instead of swapaxes, should not we?

Copy link
Contributor Author

@I8dNLo I8dNLo May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's literally the same thing, isn't it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a = np.random.random((3,4,5))
b = a.swapaxes(2, 0)
c = a.transpose((2, 0, 1))
print(a.shape, b.shape, c.shape)
>>> ((3, 4, 5), (5, 4, 3), (5, 3, 4))

d.rudenko added 3 commits May 23, 2024 15:10
# Conflicts:
#	fastembed/image/onnx_image_model.py
#	fastembed/image/transform/operators.py
@I8dNLo I8dNLo changed the title Resnet support added Draft: Resnet support added May 28, 2024
Comment on lines 99 to 134
transforms = []
cls._get_convert_to_rgb(transforms, config)
cls._get_resize(transforms, config)
cls._get_center_crop(transforms, config)
cls._get_pil2ndarray(transforms, config)
cls._get_rescale(transforms, config)
cls._get_normalize(transforms, config)
return cls(transforms=transforms)

@staticmethod
def _get_convert_to_rgb(transforms: List['Transform'], config: Dict[str, Any]):
transforms.append(ConvertToRGB())

@staticmethod
def _get_resize(transforms: List['Transform'], config: Dict[str, Any]):
mode = config.get('image_processor_type', 'CLIPImageProcessor')
if mode == 'CLIPImageProcessor':
if config.get("do_resize", False):
size = config["size"]
if "shortest_edge" in size:
size = size["shortest_edge"]
elif "height" in size and "width" in size:
size = (size["height"], size["width"])
else:
raise ValueError("Size must contain either 'shortest_edge' or 'height' and 'width'.")
transforms.append(ClipResize(size=size, resample=config.get("resample", Image.Resampling.BICUBIC)))
elif mode == 'ConvNextFeatureExtractor':
if 'size' in config and "shortest_edge" not in config['size']:
raise ValueError(f"Size dictionary must contain 'shortest_edge' key. Got {config['size'].keys()}")
shortest_edge = config['size']["shortest_edge"]
crop_pct = config.get("crop_pct", 0.875)
if shortest_edge < 384:
# maintain same ratio, resizing shortest edge to shortest_edge/crop_pct
resize_shortest_edge = int(shortest_edge / crop_pct)
transforms.append(ClipResize(size=resize_shortest_edge, resample=config.get("resample", Image.Resampling.BICUBIC)))
transforms.append(CenterCrop(size=(shortest_edge, shortest_edge)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a temporary design, we would need to come up with something better, but since it is hidden from the users, it's okay to have it this way for now

@joein
Copy link
Member

joein commented May 31, 2024

The last thing to do at the moment is to push the model to Qdrant's HF hub and change the corresponding name in image/onnx_embedding.py

@joein joein self-requested a review May 31, 2024 14:48
@joein joein merged commit 85aaae4 into qdrant:main May 31, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants