Improve model parameterization on tests #3926
Conversation
test/test_models.py
Outdated
| # The following contains configuration parameters for all models which are used by | ||
| # the _test_*_model methods. | ||
| _model_params = { | ||
| 'default_classification': { |
There was a problem hiding this comment.
nit: instead of declaring a global dict, should we instead declare one dict in each of _test_classification_model, _test_segmentation_model, etc? This would ease reading the code as relevant info would be where it's used
There was a problem hiding this comment.
Sounds good. This is how I had it originally but thought to move everything together. I'll rollback.
test/test_models.py
Outdated
| 'default_segmentation': { | ||
| 'num_classes': 10, | ||
| 'pretrained_backbone': False, | ||
| 'input_shape': (1, 3, 32, 32), |
There was a problem hiding this comment.
nit: looks like we don't need to tweak input_shape for segmentation and detection models, so I would leave it out for these. In fact I would even leave it out for the classification ones because we can just move
input_shape = (1, 3, 299, 299) if model_name == 'inception_v3' else (1, 3, 224, 224)in _test_classification_model, and it's not a model param strictly speaking, unlike the rest
There was a problem hiding this comment.
I need to reduce the size of some object detection models to speed them up (follow up PR), so I thought to introduce it in the dictionary always. The segmentation won't need any overwrites at this point.
Given this info, do you still recommend removing it from the params?
There was a problem hiding this comment.
what I meant to say is that this:
input_shape = kwargs.pop('input_shape')is a little awkward so we might want to remove those from the dict. If we do, we wouldn't need to remove e.g. input_shape = (1, 3, 32, 32) line 105.
But again this is a nit, and perhaps I'm missing something so feel free to leave as-is
Summary: * Improve model parameterization on tests. * Code review changes. Reviewed By: NicolasHug Differential Revision: D29027295 fbshipit-source-id: f3f78bc63fb4dfc2f50d931afa3bfa60fca632fb
Fixes #3891