diff --git a/hoomd/logging.py b/hoomd/logging.py index f2f43d936e..56d2dc6d47 100644 --- a/hoomd/logging.py +++ b/hoomd/logging.py @@ -383,23 +383,23 @@ def log(func=None, loggable quantity, defaults to 'scalar'. See `LoggerCategories` for available types. Keyword only argument. default (`bool`, optional): Whether the quantity should be logged - by default, defaults to True. This is orthogonal to the loggable - quantity's type. An example would be performance orientated loggable - quantities. Many users may not want to log such quantities even - when logging other quantities of that type. The default category - allows for these to be pass over by `Logger` objects by default. - Keyword only argument. + by default. Defaults to True. This is orthogonal to the loggable + quantity's type. Many users may not want to log such quantities even + when logging other quantities of that type. An example would be + performance orientated loggable quantities, like the number of + neighborlist builds. The `default` argument allows for these to be + skipped by `Logger` objects by default. Keyword only argument. requires_run (`bool`, optional): Whether this property requires the simulation to run before being accessible. Note: The namespace (where the loggable object is stored in the `Logger` - object's nested dictionary, is determined by the module/script and class + object's nested dictionary) is determined by the module/script and class name the loggable class comes from. In creating subclasses of `hoomd.custom.Action`, for instance, if the module the subclass is defined in is ``user.custom.action`` and the class name is ``Foo`` then the namespace used will be ``('user', 'custom', 'action', 'Foo')``. This - helps to prevent naming conflicts, and automate the logging + helps to prevent naming conflicts and automates the logging specification for developers and users. Example:: @@ -563,16 +563,17 @@ class they come from. For instance, the `hoomd.md.pair.LJ` class has a `Logger` objects support two ways of discriminating what loggable quantities they will accept: ``categories`` and ``only_default`` (the constructor - arguments). Both of these are static meaning that once instantiated a + arguments). Both of these are static, meaning that once instantiated, a `Logger` object will not change the values of these two properties. - ``categories`` determines what if any types of loggable quantities (see + ``categories`` determines what 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 - quantities you want in `add`, but the same is not true with regards - to ``categories``. + ``only_default`` flag prevents rarely-used quantities (e.g. the number of + neighborlist builds) from being added to the logger when + using`Logger.__iadd__` and `Logger.add` without specifying them explicitly. + In `Logger.add`, you can override the ``only_default`` flag by explicitly + listing the quantities you want to add. On the other hand, ``categories`` is + rigidly obeyed as it is a promise to logging backends. Note: The logger provides a way for users to create their own logger back @@ -595,9 +596,9 @@ 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``. Non-default quantities + are typically measures of operation performance rather than + information about simulation state. """ def __init__(self, categories=None, only_default=True): @@ -624,11 +625,12 @@ def only_default(self): quantities.""" return self._only_default - def _filter_quantities(self, quantities): + def _filter_quantities(self, quantities, force_quantities=False): for quantity in quantities: - if self._only_default and not quantity.default: + if quantity.category not in self._categories: continue - elif quantity.category in self._categories: + # Must be before default check to overwrite _only_default + if not self._only_default or quantity.default or force_quantities: yield quantity def _get_loggables_by_name(self, obj, quantities): @@ -643,7 +645,7 @@ def _get_loggables_by_name(self, obj, quantities): "object {} has not loggable quantities {}.".format( obj, bad_keys)) yield from self._filter_quantities( - map(lambda q: obj._export_dict[q], quantities)) + map(lambda q: obj._export_dict[q], quantities), True) def add(self, obj, quantities=None, user_name=None): """Add loggables from obj to logger. @@ -727,10 +729,12 @@ def __setitem__(self, namespace, value): name and category. If using a method it should not take arguments or have defaults for all arguments. """ - if isinstance(value, _LoggerEntry): - super().__setitem__(namespace, value) - else: - super().__setitem__(namespace, _LoggerEntry.from_tuple(value)) + if not isinstance(value, _LoggerEntry): + value = _LoggerEntry.from_tuple(value) + if value.category not in self.categories: + raise ValueError( + "User specified loggable is not of an accepted category.") + super().__setitem__(namespace, value) def __iadd__(self, obj): """Add quantities from object or list of objects to logger. diff --git a/hoomd/pytest/test_logging.py b/hoomd/pytest/test_logging.py index 64c07f2142..7710b2a7fa 100644 --- a/hoomd/pytest/test_logging.py +++ b/hoomd/pytest/test_logging.py @@ -2,7 +2,7 @@ # Part of HOOMD-blue, released under the BSD 3-Clause License. from hoomd.conftest import pickling_check -from pytest import raises, fixture +from pytest import raises, fixture, mark from hoomd.logging import (_LoggerQuantity, _SafeNamespaceDict, Logger, dict_map, Loggable, LoggerCategories, log) @@ -56,6 +56,10 @@ def prop(self): def proplist(self): return [1, 2, 3] + @log(category="string", default=False) + def prop_nondefault(self): + return "foo" + def __eq__(self, other): return isinstance(other, type(self)) @@ -81,7 +85,7 @@ def propnotinherented(self): not_dummy_loggable_inher = NotInherentedDummy def test_logger_functor_application(self): - loggable_list = ['prop', 'proplist'] + loggable_list = ['prop', 'proplist', "prop_nondefault"] assert set( self.dummy_loggable._export_dict.keys()) == set(loggable_list) expected_namespace = _LoggerQuantity._generate_namespace( @@ -109,7 +113,11 @@ def test_loggable_inherentence(self): def test_loggables(self): dummy_obj = self.dummy_loggable() - assert dummy_obj.loggables == {'prop': 'scalar', 'proplist': 'sequence'} + assert dummy_obj.loggables == { + 'prop': 'scalar', + 'proplist': 'sequence', + 'prop_nondefault': 'string' + } # ------- Test dict_map function @@ -187,9 +195,21 @@ def test_len(self, namespace_dict, blank_namespace_dict): # ------ Test Logger -@fixture -def blank_logger(): - return Logger() +@fixture(params=( + {}, + { + "only_default": False + }, + { + "categories": ("scalar", "string") + }, + { + "only_default": False, + "categories": ("scalar",) + }, +)) +def blank_logger(request): + return Logger(**request.param) @fixture @@ -207,20 +227,54 @@ def base_namespace(): return ('pytest', 'test_logging', 'DummyLoggable') +def nested_getitem(obj, namespace): + for k in namespace: + obj = obj[k] + return obj + + class TestLogger: + def get_filter(self, logger, overwrite_default=False): + + def filter(loggable): + with_default = not logger.only_default or loggable.default + return (loggable.category in logger.categories + and (with_default or overwrite_default)) + + return filter + def test_setitem(self, blank_logger): - logger = blank_logger - logger['a'] = (5, '__eq__', 'scalar') - logger[('b', 'c')] = (5, '__eq__', 'scalar') - logger['c'] = (lambda: [1, 2, 3], 'sequence') + + def check(logger, namespace, loggable): + if LoggerCategories[loggable[-1]] not in logger.categories: + with raises(ValueError): + logger[namespace] = loggable + return + logger[namespace] = loggable + assert namespace in logger + log_quantity = nested_getitem(logger, namespace) + assert log_quantity.obj == loggable[0] + if len(loggable) == 3: + assert log_quantity.attr == loggable[1] + assert log_quantity.category == LoggerCategories[loggable[-1]] + + # Valid values with potentially incompatible categories + check(blank_logger, 'a', (5, '__eq__', 'scalar')) + check(blank_logger, ('b', 'c'), (5, '__eq__', 'scalar')) + check(blank_logger, 'c', (lambda: [1, 2, 3], 'sequence')) + # Invalid values for value in [dict(), list(), None, 5, (5, 2), (5, 2, 1)]: with raises(ValueError): - logger[('c', 'd')] = value + blank_logger[('c', 'd')] = value + # Existent key + extant_key = next(iter(blank_logger.keys())) + # Requires that scalar category accepted or raises a ValueError with raises(KeyError): - logger['a'] = (lambda: [1, 2, 3], 'sequence') + blank_logger[extant_key] = (lambda: 1, 'scalar') def test_add_single_quantity(self, blank_logger, log_quantity): + # Assumes "scalar" is always accepted blank_logger._add_single_quantity(None, log_quantity, None) namespace = log_quantity.namespace + (log_quantity.name,) assert namespace in blank_logger @@ -235,19 +289,33 @@ def test_add_single_quantity(self, blank_logger, log_quantity): def test_get_loggables_by_names(self, blank_logger, logged_obj): # Check when quantities is None - log_quanities = blank_logger._get_loggables_by_name(logged_obj, None) - logged_names = ['prop', 'proplist'] + log_quantities = list( + blank_logger._get_loggables_by_name(logged_obj, None)) + log_filter = self.get_filter(blank_logger) + logged_names = [ + loggable.name + for loggable in logged_obj._export_dict.values() + if log_filter(loggable) + ] + assert len(log_quantities) == len(logged_names) assert all([ - log_quantity.name in logged_names for log_quantity in log_quanities + log_quantity.name in logged_names for log_quantity in log_quantities ]) # Check when quantities is given - accepted_quantities = ['prop', 'proplist'] - log_quanities = blank_logger._get_loggables_by_name( - logged_obj, accepted_quantities) + accepted_quantities = ['proplist', "prop_nondefault"] + log_filter = self.get_filter(blank_logger, overwrite_default=True) + log_quantities = list( + blank_logger._get_loggables_by_name(logged_obj, + accepted_quantities)) + logged_names = [ + loggable.name + for loggable in logged_obj._export_dict.values() + if loggable.name in accepted_quantities and log_filter(loggable) + ] + assert len(log_quantities) == len(logged_names) assert all([ - log_quantity.name in accepted_quantities - for log_quantity in log_quanities + log_quantity.name in logged_names for log_quantity in log_quantities ]) # Check when quantities has a bad value @@ -256,60 +324,38 @@ def test_get_loggables_by_names(self, blank_logger, logged_obj): a = blank_logger._get_loggables_by_name(logged_obj, bad_quantities) list(a) - def test_add(self, blank_logger, logged_obj, base_namespace): - - # Test adding everything - blank_logger.add(logged_obj) - expected_namespaces = [ - base_namespace + ('prop',), base_namespace + ('proplist',) - ] - assert all(ns in blank_logger for ns in expected_namespaces) - assert len(blank_logger) == 2 + @mark.parametrize("quantities", ([], [ + "prop", + ], ['prop', 'proplist', "prop_nondefault"])) + def test_add(self, blank_logger, logged_obj, base_namespace, quantities): - # Test adding specific quantity - blank_logger._dict = dict() - blank_logger.add(logged_obj, 'prop') - expected_namespace = base_namespace + ('prop',) - assert expected_namespace in blank_logger - assert len(blank_logger) == 1 + if len(quantities) != 0: + blank_logger.add(logged_obj, quantities) + log_filter = self.get_filter(blank_logger, overwrite_default=True) + else: + blank_logger.add(logged_obj) + log_filter = self.get_filter(blank_logger) - # Test multiple quantities - blank_logger._dict = dict() - blank_logger.add(logged_obj, ['prop', 'proplist']) expected_namespaces = [ - base_namespace + ('prop',), base_namespace + ('proplist',) + base_namespace + (loggable.name,) + for loggable in logged_obj._export_dict.values() + if log_filter(loggable) ] - assert all([ns in blank_logger for ns in expected_namespaces]) - assert len(blank_logger) == 2 - - # Test with category - blank_logger._dict = dict() - blank_logger._categories = LoggerCategories['scalar'] - blank_logger.add(logged_obj) - expected_namespace = base_namespace + ('prop',) - assert expected_namespace in blank_logger - assert len(blank_logger) == 1 + if len(quantities) != 0: + expected_namespaces = [ + name for name in expected_namespaces + if any(name[-1] in q for q in quantities) + ] + assert all(ns in blank_logger for ns in expected_namespaces) + assert len(blank_logger) == len(expected_namespaces) - def test_add_with_user_names(self, blank_logger, logged_obj, - base_namespace): + def test_add_with_user_names(self, logged_obj, base_namespace): + logger = Logger() # Test adding a user specified identifier into the namespace user_name = 'UserName' - blank_logger.add(logged_obj, user_name=user_name) - assert base_namespace[:-1] + (user_name, 'prop') in blank_logger - assert base_namespace[:-1] + (user_name, 'proplist') in blank_logger - - def test_add_with_categories(self, blank_logger, logged_obj, - base_namespace): - blank_logger._categories = LoggerCategories['scalar'] - # Test adding everything should filter non-scalar - blank_logger.add(logged_obj) - expected_namespace = base_namespace + ('prop',) - assert expected_namespace in blank_logger - blank_logger._categories = LoggerCategories['sequence'] - expected_namespace = base_namespace + ('proplist',) - blank_logger.add(logged_obj) - assert expected_namespace in blank_logger - assert len(blank_logger) == 2 + logger.add(logged_obj, user_name=user_name) + assert base_namespace[:-1] + (user_name, 'prop') in logger + assert base_namespace[:-1] + (user_name, 'proplist') in logger def test_remove(self, logged_obj, base_namespace): @@ -359,13 +405,13 @@ def test_remove(self, logged_obj, base_namespace): assert prop_namespace[:-2] + (prop_namespace[-2] + '_1', prop_namespace[-1]) not in log - def test_remove_with_user_name(self, blank_logger, logged_obj, - base_namespace): + def test_remove_with_user_name(self, logged_obj, base_namespace): # Test remove using a user specified namespace identifier + logger = Logger() user_name = 'UserName' - blank_logger.add(logged_obj, user_name=user_name) - assert base_namespace[:-1] + (user_name, 'prop') in blank_logger - assert base_namespace[:-1] + (user_name, 'proplist') in blank_logger + logger.add(logged_obj, user_name=user_name) + assert base_namespace[:-1] + (user_name, 'prop') in logger + assert base_namespace[:-1] + (user_name, 'proplist') in logger def test_iadd(self, blank_logger, logged_obj): blank_logger.add(logged_obj) @@ -373,7 +419,13 @@ def test_iadd(self, blank_logger, logged_obj): blank_logger._dict = dict() blank_logger += logged_obj assert add_log == blank_logger._dict - assert len(blank_logger) == 2 + log_filter = self.get_filter(blank_logger) + expected_loggables = [ + loggable.name + for loggable in logged_obj._export_dict.values() + if log_filter(loggable) + ] + assert len(blank_logger) == len(expected_loggables) def test_isub(self, logged_obj, base_namespace): diff --git a/hoomd/pytest/test_table.py b/hoomd/pytest/test_table.py index d67cc3731a..5806436f08 100644 --- a/hoomd/pytest/test_table.py +++ b/hoomd/pytest/test_table.py @@ -32,7 +32,7 @@ def __eq__(self, other): @pytest.fixture def logger(): - logger = hoomd.logging.Logger(categories=['scalar']) + logger = hoomd.logging.Logger(categories=['scalar', "string"]) logger[('dummy', 'loggable', 'int')] = (Identity(42000000), 'scalar') logger[('dummy', 'loggable', 'float')] = (Identity(3.1415), 'scalar') logger[('dummy', 'loggable', 'string')] = (Identity("foobarbaz"), 'string') diff --git a/sphinx-doc/conf.py b/sphinx-doc/conf.py index d940da008c..38e3f14f84 100644 --- a/sphinx-doc/conf.py +++ b/sphinx-doc/conf.py @@ -59,7 +59,7 @@ version = '3.2.0' release = '3.2.0' -language = None +language = "en" default_role = 'any'