search

KamitaniLab / bdpy

Python package for brain decoding analysis (BrainDecoderToolbox2 data format, machine learning analysis, functional MRI)
MIT License
33 stars 22 forks source link

Fix issue #72 Forward Hooks Persist After Destroying FeatureExtractor #91

Closed ShunsukeOnoo closed 1 month ago

ShunsukeOnoo commented 1 month ago

This pull request fixes issue #72 where forward hooks on the encoder persist after destroying the FeatureExtractor. This issue caused unnecessary features to be stored incrementally, consuming RAM if the same encoder was used for another reconstruction. This pull request adds a destructor to the FeatureExtractor class that removes all forward hooks added by it. With these updates, the same encoder can be used for reconstruction multiple times without needing to be reloaded.

ShuntaroAoki commented 1 month ago

Thank you for the PR. This is great! I'm readly to merge it. However, there is one concern: this will remove all forward hooks registered before the FeatureExtractor instance. Although that might be very rare situation and we could just ignore it, I'd like to avoid such a side effect as much as possible. Could you fix the code to only remove hooks that were added by the instance, or at least raise a warning to notify users that all hooks are removed?

ShunsukeOnoo commented 1 month ago

Thank you for your comments. I modified a bit so that only the forward hooks added by the extractor will be removed. I have also found that my original code didn't work when using layer_mapping, so I fixed it as well.