Let user know which tags are missing per run #967
Conversation
Some early users of margin plots noticed a bug. If data associated with a tag for any run was missing, an error message appeared noting that data was missing for that tag. The error message gave no indication of which runs were missing information. Sometimes, the behavior is intended. For instance, some run might not log a certain scalar value. This change now offers users a more nuanced message when margin plot logic is missing information for certain tags within a run. The message notes specific runs and then lists the specific tags without data for those runs. The message is placed under a collapsible so that the user can chose to not see the message if the behavior is intended.
| * values) that are missing for that run. A single run may be missing up | ||
| * to 3 scalar series (for value, lower, and upper). | ||
| */ | ||
| _tagsToMissingRuns: { |
There was a problem hiding this comment.
Wouldn't runsToMissingTags make more sense as a name? Since it's runs mapped to tags, not tags mapped to runs, and the tags rather than the runs which are missing.
There was a problem hiding this comment.
Ah thanks! Did away with this property.
| /** | ||
| * This object maps a run to a list of tags (of entries of scalar | ||
| * values) that are missing for that run. A single run may be missing up | ||
| * to 3 scalar series (for value, lower, and upper). |
There was a problem hiding this comment.
Hmm, wait so does this mean that we show a "missing tags" error for any run at all that doesn't have all three tags? That definitely seems like too noisy an error - it seems like it would appear for almost any typical usage of the margin charts functionality unless every scalar tag is part of a margin plot.
My understanding was that these warnings were to tell you only when there's a case where a run has some of the necessary tags but not all of them. So it would make sense to me to only say a run is "missing" tags if it has at least 1 of the three defined tags. In that sense, it could be missing at most 2 other tags.
There was a problem hiding this comment.
+1, we now only show warnings if a run is missing some but not all tags.
| <div class="error-content"> | ||
| <iron-icon class="error-icon" icon="icons:error"></iron-icon> | ||
| <template is="dom-repeat" | ||
| items="[[_enumeratedMissingTags]]" |
There was a problem hiding this comment.
Rather than having _enumeratedMissingTags defined as a separate property (which was confusing me since it seemed like basically the same data as the tagsToMissingRuns property), maybe we can just locally convert it here with a generic helper? Like this: https://stackoverflow.com/a/30794220/1179226
There was a problem hiding this comment.
Refactored per other comment.
| this._tagsToMissingRuns[run] = tagsNotFound; | ||
| // We trigger notifications in polymer by creating a new object. | ||
| // Polymer's property observers do not work for properties with | ||
| // names containing periods, and run names may have periods. |
There was a problem hiding this comment.
Hmm, it's a bit unfortunate to not be able to use the native mutation notification logic. I wonder if we could work around this by instead storing enumeratedMissingTags directly (so in contravention of my comment above) and then doing the mutations on that using the built-in array mutation methods.
The only caveat would be not being able to look up a run's missing tag entry directly by the run name (one would instead have to iterate through the array). But we could fix that by just keeping a separate runToMissingTagsIndex property that maps run names to indexes in the array. (In fact, I think we could even technically use the same property object by just adding the runs as properties on the array object, though maybe that's a little too hacky.)
There was a problem hiding this comment.
Using array mutations works!
Let user know which tags are missing per run (tensorflow#967)
Some early users of margin plots noticed a bug. If data associated with
a tag for any run was missing, an error message appeared noting that
data was missing for that tag. The error message gave no indication of
which runs were missing information.
Sometimes, the behavior (missing information) is intended. For instance,
some run might not log a certain scalar value.
This change now offers users a more nuanced message when margin plot
logic is missing information for certain tags within a run. The message
notes specific runs and then lists the specific tags without data for
those runs. The message is placed under a collapsible so that the user
can chose to not see the message if the behavior is intended.