Hparams: Fix metric info generation for sessions without run names. #6541
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bmd3k
changed the title
yatbear
approved these changes
Aug 11, 2023
| session_groups=[ | ||
| provider.HyperparameterSessionGroup( | ||
| root=provider.HyperparameterSessionRun( | ||
| experiment_id="exp/session_1", run="" |
There was a problem hiding this comment.
Should the id field just be exp?
There was a problem hiding this comment.
No, it has to be the entire path of the session. I left a comment in the test trying to explain for future readers.
yatbear
approved these changes
Aug 14, 2023
bmd3k
added a commit
that referenced
this pull request
Aug 14, 2023
…s. (#6543) Generate metric values for hparams plugin `/session_groups` requests when the session groups are generated from DataProvider.read_hyperparameters(). We need to reuse the logic introduced in #6539 to generate metric_infos for each session group and also query for scalar values. We reuse the existing logic to join the two collections of data into metric_values for the `/session_groups` request. We also continue the work begun in #6541 to improve how we generate sessions - in this case also handling cases where experiment_id is not specified for the session. This became urgently necessary to address in order to get new tests in list_session_groups_test.py to work with existing test data.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Aug 25, 2023
…ensorflow#6541) Fix a bug where we couldn't generate metric infos for HyperparameterSessionRun without run names. We were failing to match the sessions with the runs returned by scalars_metadata(). As an example: When a HyperparameterSessionRun does not contain `run` field, we were generating session_names of the form 'exp1/', 'exp2/', etc.. with a trailing '/'. Meanwhile, runs would be just of the form 'exp1/run_name'. The logic to match paths in _find_longest_parent_path() would first try to find 'exp1/run_name' in session_names and then 'exp1' in session_names. The second try would fail because of the trailing slashes in session_names. So, instead, when there is no `run` field in a HyperparameterSessionRun, we drop the final '/' and just generate names like 'exp1', 'exp2', etc... and the algorithm in _find_longest_parent_path() succeeds.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Aug 25, 2023
…s. (tensorflow#6543) Generate metric values for hparams plugin `/session_groups` requests when the session groups are generated from DataProvider.read_hyperparameters(). We need to reuse the logic introduced in tensorflow#6539 to generate metric_infos for each session group and also query for scalar values. We reuse the existing logic to join the two collections of data into metric_values for the `/session_groups` request. We also continue the work begun in tensorflow#6541 to improve how we generate sessions - in this case also handling cases where experiment_id is not specified for the session. This became urgently necessary to address in order to get new tests in list_session_groups_test.py to work with existing test data.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix a bug where we couldn't generate metric infos for HyperparameterSessionRun without run names. We were failing to match the sessions with the runs returned by scalars_metadata().
As an example:
When a HyperparameterSessionRun does not contain
runfield, we were generating session_names of the form 'exp1/', 'exp2/', etc.. with a trailing '/'. Meanwhile, runs would be just of the form 'exp1/run_name'.The logic to match paths in _find_longest_parent_path() would first try to find 'exp1/run_name' in session_names and then 'exp1' in session_names. The second try would fail because of the trailing slashes in session_names.
So, instead, when there is no
runfield in a HyperparameterSessionRun, we drop the final '/' and just generate names like 'exp1', 'exp2', etc... and the algorithm in _find_longest_parent_path() succeeds.