Adding stop threshold logic #3863
Conversation
There was a problem hiding this comment.
I am bit worried about the behavior here since the threshold is not necessary monotonic increasing/decreasing. We might just quit early when the model is in a locally optimized situation.
I think we should consider "How long it has been above the threshold" if possible. @petermattson for more inputs.
| import tensorflow as tf | ||
|
|
||
|
|
||
| def past_stop_threshold(stop_threshold, eval_metric): |
There was a problem hiding this comment.
As discussed offline, probably should have a similar function which stop when a metric is smaller that the threshold.
There was a problem hiding this comment.
It's pretty simple to add a flag in theory, but in practice, I don't know that we have a use-case yet for a lower-is-better metric. @petermattson , is any target metric decreasing?
There was a problem hiding this comment.
For performance purposes, crossing the line is acceptable, since all the benchmark implementations play by the same rules. The only question is timing instability, and we're addressing that through multiple runs. We can revisit if that's insufficient.
There was a problem hiding this comment.
That addresses the question of monotonicity; do you also anticipate needing to handle decreasing metrics (ie, the model is better if the metric is lower), or just increasing metrics for now?
|
@petermattson -- there are a couple questions for you in the comments; can you address? |
|
Increasing only seems fine for now. We can always rephrase the metric.
Thanks!
…On Mon, Apr 9, 2018 at 1:21 PM, Karmel Allison ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In official/utils/misc/model_helpers.py
<#3863 (comment)>:
> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+# ==============================================================================
+"""Miscellaneous functions that can be called by models."""
+
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+import numbers
+
+import tensorflow as tf
+
+
+def past_stop_threshold(stop_threshold, eval_metric):
That addresses the question of monotonicity; do you also anticipate
needing to handle decreasing metrics (ie, the model is better if the metric
is lower), or just increasing metrics for now?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3863 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AhFaHaHPu2nThb6WMaojEp9O0ybx8BxNks5tm8LhgaJpZM4TF4ai>
.
|
|
Pushing this now, and then will copy over manually to add build files. |
|
Thanks Karmel! I appreciate you putting this in. :-)
…On Tue, Apr 10, 2018 at 8:37 AM, Karmel Allison ***@***.***> wrote:
Merged #3863 <#3863>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3863 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AhFaHUsR4D_O7UqcyHZTU5IX8jn4lrYgks5tnNGcgaJpZM4TF4ai>
.
|
* Adding tests * Adding tests * Repackaging * Adding logging * Linting
We want to be able to stop training at a specified accuracy or other model-specific metric. Helper function added here, along with run loop logic for existing models.
@qlzh727 -- when running time-to-converge tests, we will likely want to run with the flag --stop_threshold=
@yhliang2018 -- when adding new models, they should include stopping logic in the run loop.