-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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
Add ViTImageProcessorFast to tests #31424
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -399,7 +399,7 @@ def from_pretrained(cls, pretrained_model_name_or_path, *inputs, **kwargs): | |
kwargs["token"] = use_auth_token | ||
|
||
config = kwargs.pop("config", None) | ||
use_fast = kwargs.pop("use_fast", False) | ||
use_fast = kwargs.pop("use_fast", None) | ||
trust_remote_code = kwargs.pop("trust_remote_code", None) | ||
kwargs["_from_auto"] = True | ||
|
||
|
@@ -430,10 +430,11 @@ def from_pretrained(cls, pretrained_model_name_or_path, *inputs, **kwargs): | |
|
||
if image_processor_class is not None: | ||
# Update class name to reflect the use_fast option. If class is not found, None is returned. | ||
if use_fast and not image_processor_class.endswith("Fast"): | ||
image_processor_class += "Fast" | ||
elif not use_fast and image_processor_class.endswith("Fast"): | ||
image_processor_class = image_processor_class[:-4] | ||
if use_fast is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We only update the image_processor_class if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I got the idea. But it looks like to me here you want to make sure if a slow (rather than There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I completely understood your comment, but let me try and explain the desired behaviour, and you can let me know if I've mistaken something :) At the moment, we don't want Desired behaviours
i.e. if a fast image processor is available, but a slow image processor is saved out, we still want to default to the slow image processor for now as they won't produce exactly the same outputs, and instead inform them that a fast version is available. This is to avoid unexpected behaviour There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, I got it now.
which is not the current sorry (not easy to reason about all these cases and motivation) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yep! It was a bug that I hadn't noticed before 😬 |
||
if use_fast and not image_processor_class.endswith("Fast"): | ||
image_processor_class += "Fast" | ||
elif not use_fast and image_processor_class.endswith("Fast"): | ||
image_processor_class = image_processor_class[:-4] | ||
image_processor_class = image_processor_class_from_name(image_processor_class) | ||
|
||
has_remote_code = image_processor_auto_map is not None | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -772,7 +772,7 @@ def preprocess( | |
ignore_index, | ||
do_reduce_labels, | ||
return_tensors, | ||
input_data_format=input_data_format, | ||
input_data_format=data_format, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this model (and the ones below with this change) the "input_data_format" is now data_format because the images were changed to data_format in the _preprocess_image step. This wasn't previously caught because the test images and the defaults were always in channels_last format. Now, we're feeding in channels_first numpy images for tests, as this is the standard format for numpy images. |
||
) | ||
return encoded_inputs | ||
|
||
|
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.
@molbap Would be good to have your confirmation here that we can remove - I don't think we want to save this out in the config: it's private and wouldn't be used when creating a new config class.
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.
yes, I agree! no use having it written to the config
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.
@molbap
Just wondering:
_valid_processor_keys
instance attribute? (could be class attribute?)