Skip to content

Group colours by regex of the experiment name (UI). #6847

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
May 20, 2024
Merged

Group colours by regex of the experiment name (UI). #6847

merged 8 commits into from
May 20, 2024

Conversation

AnuarTB
Copy link
Contributor

@AnuarTB AnuarTB commented May 7, 2024

NOTE: This PR is a continuation of #6846.

Motivation for features / changes

Users want to color runs by the regex string of the corresponding to runs experiment names.

Technical description of changes

  • Added new GroupBy type, REGEX_BY_EXP.
  • Added a dropdown in dialog window, so users could select between regex for run name or experiment name.

Screenshots of UI changes (or N/A)

N/A since internal change.

Detailed steps to verify changes work correctly (as executed by you)

  • Run tensorboard.corp server.
  • Click on color grouping icon and select Regex.
  • Select Experiment Name in dropdown.

Alternate designs / implementations considered (or N/A)

N/A

@AnuarTB AnuarTB requested review from hoonji and roseayeon May 7, 2024 09:34
@AnuarTB AnuarTB mentioned this pull request May 7, 2024
Merged
@roseayeon
Copy link
Contributor

roseayeon commented May 7, 2024 •
edited
Loading

Build is failing now. Have you confirmed that TAP presubmit has passed?

Oh I think expect_angular_material_core should be included in this PR

hoonji
hoonji requested changes May 8, 2024
View reviewed changes
Copy link
Member

@hoonji hoonji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting code! Thanks for also cleaning up issues in the existing feature.

@AnuarTB
Copy link
Contributor Author

AnuarTB commented May 10, 2024

Now the only part left is to make a commit for feature flag. I will try to make it in this PR as well.

@AnuarTB AnuarTB force-pushed the regex_feature_1 branch from f6064ea to 941ecb0 Compare May 13, 2024 07:49
@AnuarTB AnuarTB requested a review from hoonji May 13, 2024 07:50
@AnuarTB AnuarTB force-pushed the regex_feature_1 branch from 941ecb0 to 2808c56 Compare May 13, 2024 08:06
roseayeon
roseayeon reviewed May 14, 2024
View reviewed changes
@AnuarTB AnuarTB force-pushed the regex_feature_1 branch from 2808c56 to 475d25f Compare May 14, 2024 04:04
hoonji
hoonji requested changes May 14, 2024
View reviewed changes
Copy link
Member

@hoonji hoonji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates! Just a few more items

@AnuarTB AnuarTB requested review from hoonji and roseayeon May 16, 2024 07:37
@AnuarTB AnuarTB force-pushed the regex_feature_1 branch 2 times, most recently from 927c819 to e9d038a Compare May 17, 2024 01:34
hoonji
hoonji reviewed May 17, 2024
View reviewed changes
hoonji
hoonji requested changes May 17, 2024
View reviewed changes
Copy link
Member

@hoonji hoonji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New changes look great, I'll approve as soon as the CI issues are fixed (please request re-review when it's green)

@AnuarTB AnuarTB force-pushed the regex_feature_1 branch from e9d038a to 4dad4dc Compare May 17, 2024 05:00
roseayeon
roseayeon approved these changes May 17, 2024
View reviewed changes
Copy link
Contributor

@roseayeon roseayeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! please merge this PR if CI passes

@AnuarTB AnuarTB requested a review from hoonji May 17, 2024 13:18
hoonji
hoonji approved these changes May 19, 2024
View reviewed changes
Copy link
Member

@hoonji hoonji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for the keras workaround!

@AnuarTB AnuarTB merged commit 7df7ad9 into tensorflow:master May 20, 2024
@arcra arcra mentioned this pull request May 21, 2024
Merged
AnuarTB pushed a commit that referenced this pull request May 21, 2024
@arcra
Adds import that was used via a transitive dependency. (#6856)
5f8b019 
This was missing when #6847 was merged, but it's causing some build
errors in internal repo due to more strict dependency checks (this
target is currently used via transitive dependencies).
AnuarTB added a commit to AnuarTB/tensorboard that referenced this pull request Jun 4, 2024
@AnuarTB
Fix the issue with coloring when trying to add more experiments.
0947e34 
There was a problem when user tried to 1) enable coloring by experiment
name, 2) trying to add more experiments. It stemmed from not
implementing the serialization and deserialization of REGEX_BY_EXP in
the query pararms of the route. To followup the tensorflow#6846 and tensorflow#6847 I have
added the 'regex_exp:' as a query parameter for coloring by experiment
name.
@AnuarTB AnuarTB mentioned this pull request Jun 4, 2024
Merged
AnuarTB added a commit to AnuarTB/tensorboard that referenced this pull request Jun 4, 2024
@AnuarTB
Fix the issue with coloring when trying to add more experiments.
388ddf5 
There was a problem when user tried to 1) enable coloring by experiment
name, 2) trying to add more experiments. It stemmed from not
implementing the serialization and deserialization of REGEX_BY_EXP in
the query pararms of the route. To followup the tensorflow#6846 and tensorflow#6847 I have
added the 'regex_exp:' as a query parameter for coloring by experiment
name.
AnuarTB added a commit to AnuarTB/tensorboard that referenced this pull request Jun 4, 2024
@AnuarTB
Fix the issue with coloring when trying to add more experiments.
6268a35 
There was a problem when user tried to 1) enable coloring by experiment
name, 2) trying to add more experiments. It stemmed from not
implementing the serialization and deserialization of REGEX_BY_EXP in
the query pararms of the route. To followup the tensorflow#6846 and tensorflow#6847 I have
added the 'regex_exp:' as a query parameter for coloring by experiment
name.
AnuarTB added a commit that referenced this pull request Jun 4, 2024
@AnuarTB
Fix the issue with coloring when trying to add more experiments. (#6861)
c3fd2b5 
There was a problem when user tried to 1) enable coloring by experiment
name, 2) trying to add more experiments. It stemmed from not
implementing the serialization and deserialization of REGEX_BY_EXP in
the query pararms of the route. To followup the #6846 and #6847 I have
added the 'regex_exp:' as a query parameter for coloring by experiment
name.

## Motivation for features / changes

To fix the issue with the new feature. 

## Technical description of changes

Added `regex_exp:` query param for serialization and deserialization of
`REGEX_BY_EXP` GroupBy. It ensures that whenever user share the URL the
filter will remain in place.

## Screenshots of UI changes (or N/A)

## Detailed steps to verify changes work correctly (as executed by you)

1) Enable the coloring by experiment name.
2) Try to add more experiments via pressing `Add more experiments` in
the top panel.

## Alternate designs / implementations considered (or N/A)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hoonji hoonji hoonji approved these changes

@roseayeon roseayeon roseayeon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AnuarTB @roseayeon @hoonji