-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Fix unnecessary reloading of the pre-trained model in EVAL and PREDICT #450
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
base: master
Are you sure you want to change the base?
Conversation
…T phases. RunHook was used to provide a clean implementation.
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
- Removed unnecessary parameter "init_checkpoint" in model_fn_builder() - Updated indentation to 2 spaces to be consistent with Google's coding style.
@peidaqi Thank for the code. However, when I run your code, the inference time (prediction phase) seems not to be reduced. Do you have any idea about this ? Below is the time logging in the prediction function: -- Your run_classifier.py -- Original run_classsifier.py
|
@ntson2002 Not sure what you mean by not reduced, but it seems to me the time difference in your reports show the new run_classifer.py is indeed faster - thought only by a small margin. estimator.predict returns a generator so I imagine most time would be spent with the Python facilities and loops. Also your code has lots of I/O per loop that takes lots of time, compared to the one-off loading of the pre-trained model, which could make the difference insignificant. I'd suggest testing with estimator.eval and remove all those I/O operations. After all, this is only a small fix, and if you have fast SSDs (seems to be in your case?) the performance gain would not be very big. |
@googlebot I signed it! |
In the current implementation, the pre-trained model will be loaded not only in TRAIN, but also in EVAL and PREDICT.
This is not necessary as the weights will be overwritten anyway when the Estimator framework loads previous check points from TRAIN in EVAL and PREDICT phases.
A RunHook was used to provide a clean solution to load the pre-trained model, as in this way the model_fn() is left with only the graph construction code. By passing the RunHook to estimator.train(), it ensures the pre-trained model gets loaded only during TRAIN. Since the RunHook will run on CPU, there's no longer needs for scaffolding, either.