CI: fix efficientnet pipeline timeout and prevent future similar issues due to large image size
#33123
Conversation
| @@ -83,6 +83,7 @@ def prepare_config_and_inputs(self): | |||
|
|
|||
| def get_config(self): | |||
| return EfficientNetConfig( | |||
| image_size=self.image_size, | |||
There was a problem hiding this comment.
This parameter was not being piped -> the tester's config had the (large) default image size -> the tiny model is created using a large image size in the processor -> pipeline test using the tiny model resulted in a timeout, because it was using a large image size
| @@ -1718,7 +1718,7 @@ | |||
| "model_classes": [ | |||
| "EfficientNetForImageClassification" | |||
| ], | |||
| "sha": "6ed195ee636d2c0b885139da8c7b45d57ebaeee0" | |||
| "sha": "993d088cf937b8a90b61f68677cd8f261321c745" | |||
There was a problem hiding this comment.
Corresponds to this commit.
This is the processor config that results from the corrected workflow.
| largest_image_size = max(feature_extractor.size["height"], feature_extractor.size["width"]) | ||
| if largest_image_size > 64: | ||
| # hardcoded exceptions | ||
| models_with_large_image_size = ("deformable_detr", "flava", "grounding_dino", "mgp_str", "swiftformer") |
There was a problem hiding this comment.
manually checked with a search using "image_size=" as a search word, on the model tester files.
There was a problem hiding this comment.
does this mean those model could run pipeline tests with larger image size, so we don't care?
There was a problem hiding this comment.
@ydshieh yeah -- that is what is happening at the moment.
For a new model, if we need to use a larger image size in the tiny model, we would have to add it here. In other words, it works as an intentional source of friction.
| @@ -1395,7 +1416,7 @@ def create_tiny_models( | |||
| raise ValueError(f"This script should be run from the root of the clone of `transformers` {clone_path}") | |||
|
|
|||
| report_path = os.path.join(output_path, "reports") | |||
| os.makedirs(report_path) | |||
| os.makedirs(report_path, exist_ok=True) | |||
There was a problem hiding this comment.
before: we had to manually remove the reports folder each time we ran the command.
now: it overwrites the contents of the reports folder when we run the command (much faster to debug things :D)
| try: | ||
| test_module = importlib.import_module(test_module_path) | ||
| except AttributeError as exc: | ||
| # e.g. if you have a `tests` folder in `site-packages`, created by another package, when trying to import | ||
| # `tests.models...` | ||
| raise ValueError( | ||
| f"Could not import module {test_module_path}. Confirm that you don't have a package with the same root " | ||
| "name installed or in your environment's `site-packages`." | ||
| ) from exc |
There was a problem hiding this comment.
importlib.import_module(test_module_path) tries to import a local module. In my particular setup, the local module ("tests") conflicted with a package that I had installed.
I took a while to figure out why it was crashing, so I decided to enhance the exception message 👀
There was a problem hiding this comment.
yeah, it happened to me before!
efficientnet pipeline timeout and prevent future similar issues
efficientnet pipeline timeout and prevent future similar issuesefficientnet pipeline timeout and prevent future similar issues due to large image size
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
utils/create_dummy_models.py
Outdated
| if largest_image_size > 64: | ||
| # hardcoded exceptions | ||
| models_with_large_image_size = ("deformable_detr", "flava", "grounding_dino", "mgp_str", "swiftformer") | ||
| if any(model_name in feature_extractor.__class__.__name__ for model_name in models_with_large_image_size): |
There was a problem hiding this comment.
feature_extractor.__class__.__name__ is the class name of a feature extractor, which is unlikely to contain model type names like deformable_detr or grounding_dino. Also the upper/lower cases should be considered.
There was a problem hiding this comment.
(or just use the class names in models_with_large_image_size)
There was a problem hiding this comment.
@ydshieh I've modified it to use tiny_config.model_type, which should be a more reliable source of the model architecture being used 🤗
Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
…sues due to large image size (huggingface#33123) * fix param not being passed in tested; add exceptions * better source of model name * Update utils/create_dummy_models.py Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com> --------- Co-authored-by: amyeroberts <22614925+amyeroberts@users.noreply.github.com>
What does this PR do?
This PR: