check min size for visual encoders #3112
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chriselion
commented
Dec 20, 2019
| @@ -427,6 +438,17 @@ def create_resnet_visual_observation_encoder( | |||
| ) | |||
| return hidden_flat | |||
|
|
|||
| @staticmethod | |||
| def get_encoder_for_type(encoder_type: EncoderType) -> EncoderFunction: | |||
| ENCODER_FUNCTION_BY_TYPE = { | |||
There was a problem hiding this comment.
Don't think I can make this dict a class member :/
ervteng
reviewed
Dec 20, 2019
ervteng
reviewed
Dec 20, 2019
| enc_func(vis_input, 32, LearningModel.swish, 1, "test", False) | ||
|
|
||
| # Anything under the min size should raise an exception. If not, decrease the min size! | ||
| with pytest.raises(Exception): |
There was a problem hiding this comment.
Should we except the specific exception (and check that it's the negative dimension issue) just in case the model has a different error?
There was a problem hiding this comment.
I don't think we need the exact type; the test is sufficient to make sure that size N works and N-1 doesn't. Besides, it's possible that different encoders would fail for different reasons.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The visual encoders have a minimum usable size due to the strides. Currently, if you go below this size, we get an ugly looking exception in keras.
This PR adds a friendlier exception when you go under the min size, and adds tests to make sure the min is actually the min.