Skip to content
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.

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

fix: Adding non-default loggables explicitly. #1331

Merged
merged 11 commits into from
May 31, 2022
11 changes: 5 additions & 6 deletions hoomd/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -568,9 +568,9 @@ class they come from. For instance, the `hoomd.md.pair.LJ` class has a
``categories`` determines what if any types of loggable quantities (see
`LoggerCategories`) are appropriate for a given `Logger` object. This helps
logging backends determine if a `Logger` object is compatible. The
``only_default`` flag is mainly a convenience by allowing quantities not
commonly logged (but available) to be passed over unless explicitly asked
for. You can override the ``only_default`` flag by explicitly listing the
``only_default`` flag affects performance by controlling whether available
Copy link
Member Author

@b-butler b-butler May 27, 2022

Choose a reason for hiding this comment

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

@cbkerr only_default doesn't really effect performance. It is just to prevent quantities that may not be commonly desired from automatically being added on Logger.__iadd__ and Logger.add.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I see the documentation was completely misleading here... I think I meant show not slow...

quantities not commonly logged are skipped (the default) or logged.
You can override the ``only_default`` flag by explicitly listing the
quantities you want in `add`, but the same is not true with regards
to ``categories``.

Expand All @@ -595,9 +595,8 @@ class they come from. For instance, the `hoomd.md.pair.LJ` class has a
the only types of loggable quantities that can be logged by this
logger. Defaults to allowing every type.
only_default (`bool`, optional): Whether to log only quantities that are
logged by "default", defaults to ``True``. This mostly means that
performance centric loggable quantities will be passed over when
logging when false.
logged by "default". Defaults to ``True``. If ``False``, loggable
Copy link
Member

Choose a reason for hiding this comment

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

Why the quotes on by "default"?
Where is this "default" specified?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is specified by the specific loggable. We could use italic. I was just trying to emphasize the word.

Copy link
Member

Choose a reason for hiding this comment

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

I think emphasizing it is confusing because the parameter also contains "default". Would we lose much by omitting this bit?

quantities that would slow performance are not be logged.
"""

def __init__(self, categories=None, only_default=True):
Expand Down