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

Failed to remove handler attached on filtered event #2684

Closed
vfdev-5 opened this issue Aug 31, 2022 · 1 comment · Fixed by #2690
Closed

Failed to remove handler attached on filtered event #2684

vfdev-5 opened this issue Aug 31, 2022 · 1 comment · Fixed by #2690
Labels

Comments

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Aug 31, 2022

🐛 Bug description

from ignite.engine import Engine, Events

e = Engine(lambda e, b: None)
assert len(e._event_handlers[Events.ITERATION_COMPLETED]) == 0

handle = e.add_event_handler(Events.ITERATION_COMPLETED(every=3), lambda _: print("Hello"))
assert len(e._event_handlers[Events.ITERATION_COMPLETED]) == 1

handle.remove()
assert len(e._event_handlers[Events.ITERATION_COMPLETED]) == 0, e._event_handlers[Events.ITERATION_COMPLETED]

Output:

---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
Input In [8], in <cell line: 11>()
      8 assert len(e._event_handlers[Events.ITERATION_COMPLETED]) == 1
     10 handle.remove()
---> 11 assert len(e._event_handlers[Events.ITERATION_COMPLETED]) == 0, e._event_handlers[Events.ITERATION_COMPLETED]

AssertionError: [(<function <lambda> at 0x132ab23a0>, (<ignite.engine.engine.Engine object at 0x13330ed00>,), {})]
> 

Environment

  • PyTorch Version (e.g., 1.4):
  • Ignite Version (e.g., 0.3.0): 0.4.9
  • OS (e.g., Linux):
  • How you installed Ignite (conda, pip, source):
  • Python version:
  • Any other relevant information:
@vfdev-5 vfdev-5 added bug module: engine Engine module labels Aug 31, 2022
@sadra-barikbin
Copy link
Collaborator

The problem rises when we construct the handle with the wrapping handler but in _compare_handlers we compare it with the parent of registered handlers.

if isinstance(event_name, CallableEventWithFilter) and event_name.filter is not None:
event_filter = event_name.filter
handler = self._handler_wrapper(handler, event_name, event_filter)

return RemovableEventHandle(event_name, handler, self)

if hasattr(registered_handler, "_parent"):
registered_handler = registered_handler._parent() # type: ignore[attr-defined]
return registered_handler == user_handler

vfdev-5 added a commit to vfdev-5/ignite that referenced this issue Sep 2, 2022
vfdev-5 added a commit that referenced this issue Sep 4, 2022
* Fixed issue when removing handlers on filtered events

Fixes #2684

* Updated tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants