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

Use weakref.proxy where parent=self. #2223

Merged
merged 3 commits into from
Jan 19, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ctapipe/core/component.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
""" Class to handle configuration for algorithms """
import warnings
import weakref
from abc import ABCMeta
from inspect import cleandoc, isabstract
from logging import getLogger
@@ -155,6 +156,8 @@ def __init__(self, config=None, parent=None, **kwargs):
with warnings.catch_warnings():
warnings.filterwarnings("error", message=".*Config option.*not recognized")
try:
if parent is not None:
parent = weakref.proxy(parent)
super().__init__(parent=parent, config=config, **kwargs)
except UserWarning as e:
raise TraitError(e) from None
4 changes: 2 additions & 2 deletions ctapipe/io/tests/test_event_source.py
Original file line number Diff line number Diff line change
@@ -117,14 +117,14 @@ def __init__(self, config=None, parent=None):
parent = Parent(config=config)

assert isinstance(parent.source, SimTelEventSource)
assert parent.source.parent is parent
assert parent.source.parent.__weakref__ is parent.__weakref__

# test with EventSource as subconfig of parent
config = Config({"Parent": {"EventSource": {"input_url": dataset}}})

parent = Parent(config=config)
assert isinstance(parent.source, SimTelEventSource)
assert parent.source.parent is parent
assert parent.source.parent.__weakref__ is parent.__weakref__


def test_from_config_default():
3 changes: 3 additions & 0 deletions ctapipe/reco/sklearn.py
Original file line number Diff line number Diff line change
@@ -2,6 +2,7 @@
Component Wrappers around sklearn models
"""
import pathlib
import weakref
from abc import abstractmethod
from collections import defaultdict

@@ -204,6 +205,8 @@ def read(cls, path, **kwargs):
instance = joblib.load(f)

for attr, value in kwargs.items():
if attr == "parent":
value = weakref.proxy(value)
setattr(instance, attr, value)

if not isinstance(instance, cls):
10 changes: 10 additions & 0 deletions docs/changes/2223.maintenance.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Use ``weakref.proxy(parent)`` in ``Component.__init__``.

Due to the configuration systems, children need to reference their parent(s).
When parents get out of scope, their children still hold the reference to them.
That means that python cannot garbage-collect the parents (which are Tools, most of the time).

This change uses weak-references (which do not increase the reference count),
which means parent-Tools can get garbage collected by python.

This decreases the memory consumption of the tests by roughly 50%.