search

yoshitomo-matsubara / torchdistill

A coding-free framework built on PyTorch for reproducible deep learning studies. 🏆25 knowledge distillation methods presented at CVPR, ICLR, ECCV, NeurIPS, ICCV, etc are implemented so far. 🎁 Trained models, training logs and configurations are available for ensuring the reproducibiliy and benchmark.
https://yoshitomo-matsubara.net/torchdistill/
MIT License
1.37k stars 132 forks source link

[BUG] Importing ForwardHookManager messes up logging to file globally #465

Closed nirgoren closed 4 months ago

nirgoren commented 5 months ago

Describe the bug Importing ForwardHookManager via from torchdistill.core.forward_hook import ForwardHookManager causes logging to file not to work properly.

import logging
from torchdistill.core.forward_hook import ForwardHookManager

logger = logging.getLogger(__name__)
logging.basicConfig(filename='test.log', encoding='utf-8', level=logging.DEBUG)
logging.debug('This message should go to the log file')
logger.debug('This should also go to the log file')

The messages do not go to the log file. Commenting out from torchdistill.core.forward_hook import ForwardHookManager, the messages go to the log file as expected.

To Reproduce Provide

  1. Exact command to run your code - provided above
  2. Whether or not you made any changes in Python code (if so, how you made the changes?) - Did not make changes
  3. YAML config file - not relevant
  4. Log file - not relevant

Expected behavior A clear and concise description of what you expected to happen. The messages should go the log file, global logging behaviour should not change due to the import.

Environment (please complete the following information):

Additional context Probably related to the logging configuration in torchdistill/common/constant.py

yoshitomo-matsubara commented 5 months ago

Hi @nirgoren ,

Thank you for the feedback.

The behavior is expected. As mentioned, torchdistill.core.forward_hook indirectly imports torchdistill/common/constant.py and sets basicConfig.

I am currently on leave and will think about some workaround.

yoshitomo-matsubara commented 4 months ago

Hi @nirgoren

It should be resolved now. The update will be available in the next PyPI package release.

Feel free to re-open the issue if it doesn't resolve your issue.