-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Tensorboard ignores misspelled flags #329
Comments
I agree. The technical problem is that the TF @jart thoughts? |
How about simply raising an AttributeError in case unparsed are not empty - similar to what you already do in case there is a missing parameter you expect: https://github.com/tensorflow/tensorflow/blob/4acb96a9708a7d791e5d005e22971a05b56adcbd/tensorflow/python/platform/flags.py#L50. It would be very similar to what argparse does by default when you call parse_args() rather than parse_known_args(). From https://svn.python.org/projects/python/branches/release27-maint/Lib/argparse.py: # =====================================
# Command line argument parsing methods
# =====================================
def parse_args(self, args=None, namespace=None):
args, argv = self.parse_known_args(args, namespace)
if argv:
msg = _('unrecognized arguments: %s')
self.error(msg % ' '.join(argv))
return args |
Certainly, that would work—but that code is owned by TensorFlow, not TensorBoard. As far as I can see, making this change would require monkey-patching the code ( |
Duplicate of tensorflow/tensorflow#11195 since TensorBoard just uses tf.flags. That issue, fortunately, should now be fixed by tensorflow/tensorflow@2652704. I think your TensorBoard should pick up the fixed behavior if you just update TensorFlow to a release that includes that commit (e.g. the tf-nightly package on PyPI should suffice). |
Tensorboard silently ignores misspelled flags. Whatever flag you pass to tensorboard it swallows it without even blinking. This might seem like a small problem, but it causes some real-life problems.
A real-life example - this caused significant cost increase in our case for our tensorboard installation in GCP. We run it in kubernetes cluster and we used the https://github.com/GoogleCloudPlatform/appengine-tensorboard project as a base for our installation. Unfortunately some time recently the name of --reload-frequency parameter has changed to --reload-interval (which is BTW. much better name for what it was) but the appengine-tensorboard project have still the old parameter in configuration (I created pull request GoogleCloudPlatform/appengine-tensorboard#1 to fix it).
Due to silent swallowing of misspelled parameters, we have not even noticed that the parameter name has changed since the appengine-tensorboard project was released (actually there was even weird inconsistency in the appengine-tensorboard project that in one place RELOAD_INTERVAL variable was mentioned but not used).
All-in-all - it would have been much better if tensorboard - following good practice of about few hundred thousands of other projects ;) - would fail if you specify wrong parameter.
The text was updated successfully, but these errors were encountered: