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

Unused object warning #1536

Merged
merged 5 commits into from
Jun 5, 2024
Merged

Unused object warning #1536

merged 5 commits into from
Jun 5, 2024

Conversation

mstimberg
Copy link
Member

This addresses the issue in #1499, by raising a warning if an object gets garbage collected but has never been included in any Network. Instead of dealing with the complexities of weak references and the InstanceTracker, as initially proposed in #1499, this simply uses the __del__ function in BrianObject. This seems to be working well, and running it against our examples I did not encounter any issues. There are false positives for "artificial" examples like https://brian2.readthedocs.io/en/stable/examples/synapses.efficient_gaussian_connectivity.html, since they create Synapses but do not ever run a simulation. In principle, we could have another check that only triggers these warnings when there has been a run at some point, but I am not sure whether it is worth the complication.
This code from the forum (where the define_network function only returns the NeuronGroups, but not the Synapses) is a good example for the kind of code that will trigger a warning: https://brian.discourse.group/t/synapses-not-functioning/1118

Copy link
Member

@thesamovar thesamovar left a comment

Choose a reason for hiding this comment

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

This seems fine to me, although I have to admit I didn't entirely understand the __del__ logic.

@mstimberg
Copy link
Member Author

This seems fine to me, although I have to admit I didn't entirely understand the __del__ logic.

The reasoning is: objects fill in their _network attribute when they are simulated as part of a Network – this is used for the errors when an object has been simulated as part of different networks. We therefore raise the warning when an object gets garbage collected when an object has its attribute _network still as None (for some reason it can happen that the attribute does not exist at all, but we can simply ignore that).

The second check is to avoid that objects such as StateUpdater, Resetter, etc. also raise this warning. It's a bit of a hack, we simply assume that everything that has a group attribute is something dependent on other objects and not worth warning about. The other alternative would be to overwrite the __del__ method in all the subclasses. We actually need to do this for Subgroup, because it stores its reference to the main group as source, not as group 🙃

@mstimberg mstimberg merged commit 30a036d into master Jun 5, 2024
62 checks passed
@mstimberg mstimberg deleted the unused_object_warning branch June 5, 2024 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants