Skip to content

Add logging of mappings in BaseLogger#3295

Closed
nowtryz wants to merge 2 commits intopytorch:masterfrom
nowtryz:master
Closed

Add logging of mappings in BaseLogger#3295
nowtryz wants to merge 2 commits intopytorch:masterfrom
nowtryz:master

Conversation

@nowtryz
Copy link
Copy Markdown

@nowtryz nowtryz commented Oct 17, 2024

Fixes #3294

Description:
Adds support for Mapping metrics in loggers

Check list:

  • New tests are added (if a new feature is added)
  • New doc strings: description and/or example code are in RST format
  • Documentation is updated (if required)

@github-actions github-actions bot added the module: handlers Core Handlers module label Oct 17, 2024
@nowtryz
Copy link
Copy Markdown
Author

nowtryz commented Oct 17, 2024

Before testing this, I would like to be sure the feature is of interest.

@nowtryz
Copy link
Copy Markdown
Author

nowtryz commented Oct 22, 2024

@vfdev-5 is it ok for you like that? If so, I'll fix the linting and adapt the tests

@nowtryz nowtryz requested a review from vfdev-5 October 22, 2024 19:59
Copy link
Copy Markdown
Collaborator

@vfdev-5 vfdev-5 left a comment

Choose a reason for hiding this comment

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

Thanks @nowtryz , this is going in a good direction, let's try to improve a bit more the code.

return metrics

@classmethod
def _compute_tags(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You can make this just a private helper method in this submodule, not necessarily a classmethod.

Comment on lines +169 to +172
concat_tf: Callable[..., Union[str, Tuple[str, ...]]],
log_text: bool, node: Iterable[Tuple[str, Any]],
parent_tag: Union[str, Tuple[str, ...]],
dest_dict: Dict[Any, Union[str, float, numbers.Number]]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not keen of passing dest_dict as the argument, but maybe we can't do anything better here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: handlers Core Handlers module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add the logging of dict metrics

2 participants