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

Add support for models that have different behavior at training/test time #38

Closed
williamwwang opened this issue Jul 20, 2017 · 11 comments
Closed
Assignees

Comments

@williamwwang
Copy link

williamwwang commented Jul 20, 2017

From what I have seen, the KerasModel wrapper does not support models with different behaviors at training and test time, such as dropout or batch normalization. This is because there is no opportunity to pass in keras.backend.learning_phase() as a parameter to the prediction functions (defined in line 70, 72 of foolbox/models/keras.py). Trying to predict using a model that depends on the learning phase yields an error such as
InvalidArgumentError (see above for traceback): You must feed a value for placeholder tensor 'dropout_1/keras_learning_phase' with dtype bool
Code to reproduce error:

from keras.models import Sequential
from keras.layers import Dropout
import numpy as np
from foolbox.models import KerasModel

model = Sequential([Dropout(.1, input_shape = (1,))])
fmodel = KerasModel(model, (0,1))
fmodel.batch_predictions(np.ones((1,1)))

@jonasrauber
Copy link
Member

Can't you just set the learning phase like this before running your code:

import keras
keras.backend.set_learning_phase(0)

For me, this seems to work.

You can also have a look at the example in the readme which actually uses Keras and sets the learning phase: https://github.com/bethgelab/foolbox#example

If I misunderstood the problem or you have an idea how we can improve Foolbox to make it easier to use with Keras, let us know.

@williamwwang
Copy link
Author

Oh right, forgot you can set the learning phase manually. Thanks!

@jonasrauber
Copy link
Member

Do you think it would be better to set it internally and add a parameter to KerasModel?

@williamwwang
Copy link
Author

That might be a useful feature, though I can't really imagine why a user would want to set the learning phase to 1 while using the KerasModel since it doesn't have any training methods. I think in native Keras, the predict function is implemented with the learning phase set to 0. Maybe you could add learning phase as a parameter with default value 0?

@jonasrauber
Copy link
Member

Reopening this until I added code to set the learning phase to 0 by default if it has not been set before. This will also simplify the example in the README and apparently is more intuitive for Keras users.

@jonasrauber jonasrauber reopened this Jul 21, 2017
@jonasrauber jonasrauber self-assigned this Jul 21, 2017
@jonasrauber
Copy link
Member

Looked into this in more detail. Unfortunately, keras does not make it as easy as I thought to set the learning phase from within foolbox because constant learning phase values must be set before creating the keras model. Instead, I would need to feed the learning phase value for every function call and handle several cases separately. I don't think it's worth adding this complexity at the moment, so the recommended way is to just set the learning phase after the keras import as done in the resnet example in the readme.

@AngusG
Copy link

AngusG commented Aug 4, 2017

I almost filed a similar issue for the TensorFlow model wrapper, but tf.placeholder_with_default does the job for things like dropout and batch norm.

@wielandbrendel
Copy link
Member

@AngusG Can you explain in more detail?

@AngusG
Copy link

AngusG commented Aug 4, 2017

It was just a comment that since feed_dict isn't exposed by the wrapper, an easy workaround if you just need to set a dropout rate or pass a state to the model which is different than in training, is to use the tf.placeholder_with_default mechanism. This was enough for my immediate needs, but it could be nice to let the user provide their own feed_dict for some use cases. I'm not sure what the best way to do that would be while maintaining a framework-neutral API.

@wielandbrendel
Copy link
Member

In principle we could expose this as an argument during the initialisation of the TF model. @jonasrauber What do you think?

@jonasrauber
Copy link
Member

@AngusG @wielandbrendel I am not sure I understand the use case. The TensorFlowModel accepts arbitrary tensors (and thus graphs) so that one can specify everything the way one wants to have it during test time. A small example showing how additional feed_dict arguments could help would be nice (preferably in a new GitHub issue); if it's useful we could indeed add this functionality.

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

No branches or pull requests

4 participants