From bb1bb370081342737a3e9278bb5fb6b8c90e5ed8 Mon Sep 17 00:00:00 2001 From: Marcel Stimberg Date: Tue, 4 Jun 2024 10:39:11 +0200 Subject: [PATCH 1/5] Remove messages out of creation stack --- brian2/core/base.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/brian2/core/base.py b/brian2/core/base.py index 72cf5f217..0c1dee37f 100644 --- a/brian2/core/base.py +++ b/brian2/core/base.py @@ -80,13 +80,8 @@ def __init__( creation_stack.append(s) creation_stack = [""] + creation_stack #: A string indicating where this object was created (traceback with any parts of Brian code removed) - self._creation_stack = ( - "Object was created here (most recent call only, full details in " - "debug log):\n" + creation_stack[-1] - ) - self._full_creation_stack = "Object was created here:\n" + "\n".join( - creation_stack - ) + self._creation_stack = creation_stack[-1] + self._full_creation_stack = "\n".join(creation_stack) if dt is not None and clock is not None: raise ValueError("Can only specify either a dt or a clock, not both.") @@ -365,11 +360,15 @@ class BrianObjectException(Exception): def __init__(self, message, brianobj): self._brian_message = message self._brian_objname = brianobj.name - self._brian_objcreate = brianobj._creation_stack + self._brian_objcreate = ( + "Object was created here (most recent call only, full details in " + "debug log):\n" + brianobj._creation_stack + ) + full_stack = "Object was created here:\n" + brianobj._full_creation_stack logger.diagnostic( "Error was encountered with object " f"'{self._brian_objname}':\n" - f"{brianobj._full_creation_stack}" + f"{full_stack}" ) def __str__(self): From f860cf8aa8566faf62e322ca1b5934c49d112966 Mon Sep 17 00:00:00 2001 From: Marcel Stimberg Date: Tue, 4 Jun 2024 10:53:54 +0200 Subject: [PATCH 2/5] Raise a warning for unused Brian objects Closes #1499 --- brian2/__init__.py | 2 +- brian2/core/base.py | 16 ++++++++++++++++ brian2/tests/test_network.py | 15 +++++++++++++++ brian2/tests/test_neurongroup.py | 30 +++++++++++++++--------------- brian2/utils/logger.py | 2 ++ 5 files changed, 49 insertions(+), 16 deletions(-) diff --git a/brian2/__init__.py b/brian2/__init__.py index f914325f6..6730aa917 100644 --- a/brian2/__init__.py +++ b/brian2/__init__.py @@ -71,7 +71,7 @@ def _check_dependencies(): ) __version_tuple__ = tuple(int(x) for x in __version__.split(".")[:3]) except ImportError: - logging.getLogger("brian2").warn( + logging.getLogger("brian2").warning( "Cannot determine Brian version, running from source and " "setuptools_scm is not installed." ) diff --git a/brian2/core/base.py b/brian2/core/base.py index 0c1dee37f..c169b1f34 100644 --- a/brian2/core/base.py +++ b/brian2/core/base.py @@ -180,6 +180,22 @@ def __init__( #: dependent objects such as `StateUpdater` add_to_magic_network = False + def __del__(self): + # For objects that get garbage collected, raise a warning if they have + # never been part of a network + if ( + getattr(self, "_network", "uninitialized") is None + and getattr(self, "group", None) is None + ): + logger.warn( + f"The object '{self.name}' is getting deleted, but was never included in a network. " + "This probably means that you did not store the object reference in a variable, " + "or that the variable was not used to construct the network.\n" + "The object was created here (most recent call only):\n" + + self._creation_stack, + name_suffix="unused_brian_object", + ) + def add_dependency(self, obj): """ Add an object to the list of dependencies. Takes care of handling diff --git a/brian2/tests/test_network.py b/brian2/tests/test_network.py index 897732a69..57351c33e 100644 --- a/brian2/tests/test_network.py +++ b/brian2/tests/test_network.py @@ -1776,6 +1776,21 @@ def test_multiple_runs_function_change(): assert_equal(mon.v[0], [1, 2, 3, 4]) +@pytest.mark.codegen_independent +def test_unused_object_warning(): + with catch_logs() as logs: + # Create a NeuronGroup that is not used in the network + NeuronGroup(1, "v:1", name="never_used") + # Make sure that it gets garbage collected + import gc + + gc.collect() + assert len(logs) == 1 + assert logs[0][0] == "WARNING" + assert logs[0][1].endswith("unused_brian_object") + assert "never_used" in logs[0][2] + + if __name__ == "__main__": BrianLogger.log_level_warn() for t in [ diff --git a/brian2/tests/test_neurongroup.py b/brian2/tests/test_neurongroup.py index f7c3947c5..53d442a15 100644 --- a/brian2/tests/test_neurongroup.py +++ b/brian2/tests/test_neurongroup.py @@ -807,7 +807,7 @@ def test_namespace_errors(): @pytest.mark.codegen_independent def test_namespace_warnings(): - G = NeuronGroup( + G1 = NeuronGroup( 1, """ x : 1 @@ -819,7 +819,7 @@ def test_namespace_warnings(): # conflicting variable in namespace y = 5 with catch_logs() as l: - G.x = "y" + G1.x = "y" assert len(l) == 1, f"got {str(l)} as warnings" assert l[0][1].endswith(".resolution_conflict") @@ -829,7 +829,7 @@ def test_namespace_warnings(): i = 5 N = 3 with catch_logs() as l: - G.x = "i // N" + G1.x = "i // N" assert len(l) == 2, f"got {str(l)} as warnings" assert l[0][1].endswith(".resolution_conflict") assert l[1][1].endswith(".resolution_conflict") @@ -838,7 +838,7 @@ def test_namespace_warnings(): del N # conflicting variables in equations y = 5 * Hz - G = NeuronGroup( + G2 = NeuronGroup( 1, """ y : Hz @@ -848,7 +848,7 @@ def test_namespace_warnings(): name=f"neurongroup_{str(uuid.uuid4()).replace('-', '_')}", ) - net = Network(G) + net = Network(G2) with catch_logs() as l: net.run(0 * ms) assert len(l) == 1, f"got {str(l)} as warnings" @@ -857,13 +857,13 @@ def test_namespace_warnings(): i = 5 # i is referring to the neuron number: - G = NeuronGroup( + G3 = NeuronGroup( 1, "dx/dt = i*Hz : 1", # unique names to get warnings every time: name=f"neurongroup_{str(uuid.uuid4()).replace('-', '_')}", ) - net = Network(G) + net = Network(G3) with catch_logs() as l: net.run(0 * ms) assert len(l) == 1, f"got {str(l)} as warnings" @@ -875,13 +875,13 @@ def test_namespace_warnings(): N = 3 i = 5 dt = 1 * ms - G = NeuronGroup( + G4 = NeuronGroup( 1, "dx/dt = x/(10*ms) : 1", # unique names to get warnings every time: name=f"neurongroup_{str(uuid.uuid4()).replace('-', '_')}", ) - net = Network(G) + net = Network(G4) with catch_logs() as l: net.run(0 * ms) assert len(l) == 0, f"got {str(l)} as warnings" @@ -893,22 +893,22 @@ def test_threshold_reset(): Test that threshold and reset work in the expected way. """ # Membrane potential does not change by itself - G = NeuronGroup(3, "dv/dt = 0 / second : 1", threshold="v > 1", reset="v=0.5") - G.v = np.array([0, 1, 2]) + G1 = NeuronGroup(3, "dv/dt = 0 / second : 1", threshold="v > 1", reset="v=0.5") + G1.v = np.array([0, 1, 2]) run(defaultclock.dt) - assert_allclose(G.v[:], np.array([0, 1, 0.5])) + assert_allclose(G1.v[:], np.array([0, 1, 0.5])) with catch_logs() as logs: - G = NeuronGroup(1, "v : 1", threshold="True") + G2 = NeuronGroup(1, "v : 1", threshold="True") assert len(logs) == 1 assert logs[0][0] == "WARNING" and logs[0][1].endswith("only_threshold") with catch_logs() as logs: - G = NeuronGroup(1, "v : 1", threshold="True", reset="") + G3 = NeuronGroup(1, "v : 1", threshold="True", reset="") assert len(logs) == 0 with catch_logs() as logs: - G = NeuronGroup(1, "v : 1", threshold="True", refractory=1 * ms) + G4 = NeuronGroup(1, "v : 1", threshold="True", refractory=1 * ms) assert len(logs) == 0 diff --git a/brian2/utils/logger.py b/brian2/utils/logger.py index 040c49226..ef0d028bf 100644 --- a/brian2/utils/logger.py +++ b/brian2/utils/logger.py @@ -482,6 +482,8 @@ def warn(self, msg, name_suffix=None, once=False): """ self._log("WARNING", msg, name_suffix, once) + warning = warn + def error(self, msg, name_suffix=None, once=False): """ Log an error message. From 72396da7799cdee39af0f0cb2f9bc9da4929ab0f Mon Sep 17 00:00:00 2001 From: Marcel Stimberg Date: Tue, 4 Jun 2024 11:17:54 +0200 Subject: [PATCH 3/5] avoid logging errors after shutdown of logging system --- brian2/utils/logger.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/brian2/utils/logger.py b/brian2/utils/logger.py index ef0d028bf..da65bbf24 100644 --- a/brian2/utils/logger.py +++ b/brian2/utils/logger.py @@ -240,6 +240,7 @@ def clean_up_logging(): Shutdown the logging system and delete the debug log file if no error occured. """ + BrianLogger.initialized = False logging.shutdown() if not BrianLogger.exception_occured and prefs["logging.delete_log_on_exit"]: if BrianLogger.tmp_log is not None: @@ -343,6 +344,9 @@ class BrianLogger: ``brian2``. """ + #: Global flag to know whether the logging system is in a usable state + initialized = False + #: Class attribute to remember whether any exception occured exception_occured = False @@ -392,6 +396,9 @@ def _log(self, log_level, msg, name_suffix, once): once : bool Whether to suppress identical messages if they are logged again. """ + if not BrianLogger.initialized: + # Prevent logging errors on exit + return name = self.name if name_suffix: name += f".{name_suffix}" @@ -713,6 +720,8 @@ def initialize(): # Handle uncaught exceptions sys.excepthook = brian_excepthook + BrianLogger.initialized = True + def get_logger(module_name="brian2"): """ From 5ab61af03580620339e2cf2c11f011f47ba4d2b5 Mon Sep 17 00:00:00 2001 From: Marcel Stimberg Date: Tue, 4 Jun 2024 11:27:14 +0200 Subject: [PATCH 4/5] garbage collect to trigger warnings --- brian2/core/network.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/brian2/core/network.py b/brian2/core/network.py index 655c745ba..4f2a44424 100644 --- a/brian2/core/network.py +++ b/brian2/core/network.py @@ -8,6 +8,7 @@ """ +import gc import os import pickle as pickle import sys @@ -1102,6 +1103,8 @@ def run( The simulation can be stopped by calling `Network.stop` or the global `stop` function. """ + # This will trigger warnings for objects that have not been included in a network + gc.collect() device = get_device() # Do not use the ProxyDevice -- slightly faster if profile is None: From 6fd792e3182a1beef8bef45a55f418695b388493 Mon Sep 17 00:00:00 2001 From: Marcel Stimberg Date: Tue, 4 Jun 2024 12:02:19 +0200 Subject: [PATCH 5/5] Do not raise unused object warnings for subgroups --- brian2/groups/subgroup.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/brian2/groups/subgroup.py b/brian2/groups/subgroup.py index 9c7823f6a..34a2a73e7 100644 --- a/brian2/groups/subgroup.py +++ b/brian2/groups/subgroup.py @@ -121,3 +121,8 @@ def __repr__(self): f"<{classname} {self.name!r} of {self.source.name!r} " f"from {self.start} to {self.stop}>" ) + + def __del__(self): + # Do not raise a warning if the subgroup gets deleted (subgroups do not need + # to be part of a network to be useful) + pass