tensorflow / tensorboard

TensorFlow's Visualization Toolkit
Apache License 2.0
6.68k stars 1.66k forks source link

Request for ability to use multiple regex's (again) #235

Open langmore opened 7 years ago

langmore commented 7 years ago

The latest version of TensorBoard dropped the ability to use multiple regex's. This issue requests that the multi-regex ability be added back.

I typically used about 10 at once. So having only 1 is insufficient.

For example:

teamdandelion commented 7 years ago

@wchargin based on discussions offline, I think we're going to add this back, yes?

EgleGrublyte commented 6 years ago

Hi, Any news about this?

nfelt commented 6 years ago

I don't have much context on how this used to work. Was this multiple regexes for filtering runs, or tags, or both? How were the multiple regexes entered? I assume the result set was items that match all the regexes (i.e. the intersection), right?

I am guessing that the main use case of multiple regexes is handling cases where matches might occur in an unknown order, otherwise if the order is known the regexes can just be concatenated with .* into a single regex, as shown in the OP's first example.

wchargin commented 6 years ago

This filtered tags. They were called “tag groups”. You can see a demo of the UI at 12:38 in the TensorBoard talk.

The implementation was deeply flawed, due to fundamental restrictions of Polymer. Tag groups were always buggy, and the bugs compounded with more features that we added, like pagination. This was one of the motivations for removing it, though in retrospect it may have been the wrong choice. I spent a good amount of time thinking about a way to fix the underlying problem (which is roughly that Polymer has no concept of persistent keys and so structural UI updates are bound to cause unnecessary unmounts and remounts), and was unable to do so without not only hand-rolling what is essentially a vdom layer but also exposing said implementation to plugin authors. Maybe I missed something, or maybe the landscape has changed in the meantime, so feel free to give it another try if you’re so inclined.

wchargin commented 6 years ago

I am guessing that the main use case of multiple regexes is handling cases where matches might occur in an unknown order, otherwise if the order is known the regexes can just be concatenated with .* into a single regex, as shown in the OP's first example.

It’s the ability, I believe, to have multiple distinct sets. You might have tag groups for .*lr=0.5.* and .*levels=3.*, and expand either or both of these at a time to see different views.

nfelt commented 6 years ago

cc @stephanwlee FYI since this kinda relates to the data-selector work

@wchargin Thanks for the background! I see, so they were independent regexes filters that each got their own category pane.

In some ways this functionality sounds like a better fit for the custom-scalars dashboard - based on the OP's request, it seems like it might be used more as a way to construct a "custom dashboard" of plots rather than on-the-fly searching. Right now each chart has to be manually specified in the custom layout, but we could add a feature where instead of "repeated Chart" the Category proto has a "tag regex" field that generates a category of regular single-line charts for all tags that match the regex. The custom scalars dashboard can't be configured from the UI, but we could enable that at some point if we resolve the Polymer element mapping issues.

As for the Polymer issue, I don't think the landscape has fundamentally changed. Last I saw from the Polymer folks on this, they basically said that by design they wanted elements to be stateless functions of their bindings so if you cared about what particular element got attached/detached, you were doing it wrong. I think we may just need to roll our own approach or do something fundamentally different since this has continued to be an obstacle.

wchargin commented 6 years ago

Last I saw from the Polymer folks on this, they basically said that by design they wanted elements to be stateless functions of their bindings so if you cared about what particular element got attached/detached, you were doing it wrong.

Yep, okay. (Lacking support for keys is still a huge problem for other reasons, but we digress.) This is kind of fundamentally a bad fit for TensorBoard, as Plottable/D3 are certainly not stateless, and certainly do care about particular DOM elements. Of course, this is also the case for lots of other general-purpose JS libraries, so the Polymer thesis seems to imply that your whole application had better be written in Polymer and have no external dependencies. But I suppose that it’s good to know what we’re dealing with, at least.

nfelt commented 6 years ago

@wchargin Yes, I agree, it's a bad fit for TensorBoard. The post I'm getting Polymer's stance from is admittedly from 2015, so maybe it's not necessarily current: https://github.com/Polymer/polymer/issues/1713#issuecomment-108729265 They did start to add keys for Polymer 2.0 (https://github.com/Polymer/polymer/pull/3970) but they backed it out because it introduced other issues. We could try talking to them to see if they have a recommendation that would actually work for our use pattern.