Introduced EventsDriven
#2107
Introduced EventsDriven
#2107
Conversation
github-actions
bot
added
docs
module: base
|
Please review this as soon as you have time, @sdesrozis @trsvchn @vfdev-5. Thanks. |
|
@KickItLikeShika Thanks for this PR! It's a tricky part, and good for you for being comfortable enough with it! I left a few comments and some tests must be fixed. |
|
@sdesrozis can you please point out to the tests that should be fixed? only the there are two failing unit test that should be fixed, usually it was working fine without any updates, see here #2106 |
|
@KickItLikeShika there are many failing tests (13) which should be fixed before merging. |
|
@vfdev-5 Yes I mean just 2 unit tests should be fixed |
ignite/engine/engine.py
Outdated
| raise TypeError( | ||
| "Argument event_name should not be a filtered event, " "please use event without any event filtering" | ||
| ) | ||
| return super(Engine, self).add_event_handler(event_name, handler, *args, **kwargs) |
There was a problem hiding this comment.
@KickItLikeShika you resolved the comment, yet you didn’t answer.
There was a problem hiding this comment.
Very sorry about that, but I answered your question here #2107 (comment), I thought you saw it and agreed so I resolved the comment, again sorry.
I think keeping the full docs in the Engine docs and minimized docs in the base class is something nice, what do you think?
|
@KickItLikeShika error in the docstring of EventDriver. one comment left. |
|
@KickItLikeShika I thought we agreed on the first PR to do only:
Here you are already modifying |
|
@vfdev-5 can you please point out to which modification in the engine you mean? I haven't changed anything in the engine except the necessary for introducing EventsDriven |
There was a problem hiding this comment.
@KickItLikeShika Thanks for the updates. Remains few things and we can merge it
There was a problem hiding this comment.
Thanks @KickItLikeShika !
Codecov failure can be ignore now.
* Updates * update docs * update docs * update docs * update docstring * update docstring * update docstring * revert * split events_driven + other updates * fix docstring * fixed deterministic issue + udpate EventsDriven * ignore type in _handler_wrapper * revert tpu-test * Revert "Removed temporary fix with mkl (#2110)" This reverts commit de4cff3. * Revert "revert tpu-test" This reverts commit 82a8d7a. * revert * Revert "revert" This reverts commit 99c3921. * revert * reverted engine + added docs for EventsDriven + fixed broken docs * updated base docs * fixed broken docs * reverted state * revert test_mixins * added tests * fixed typos
As explained here: https://github.com/pytorch/ignite/wiki/Roadmap#refactor-engine
This PR will separate the events registration code from the other code.
Check list:
Code adapted from https://github.com/vfdev-5/ignite/tree/eventdriven