I saw the comment in notebook 09_learner about how “contextlib.context_manager has a surprising “feature” which doesn’t let us raise an exception before the yield” and how _CbCtxInner should fix this, so I tried to test it by impementing a simple CB that skips a particular batch, for example:
order = Callback.order - 1
def __init__(self, n): fc.store_attr()
def before_batch(self, learner): if learner.iter == self.n: raise CancelBatchException()
From my testing this CB doesn´t work, because the CancelBatchException() is never handled.
From what I understand after some debugging, what happens is that the CancelBatchException() is raised in the __enter__ method from self.cb_ctx('batch'), but it is first catched only by the __exit__ method from self.cb_ctx('epoch') and later by the __exit__ method from self.cb_ctx('fit'). The problem is the line return exc_type==chk_exc in these methods always returns False, since CancelBatchException() != CancelEpochException() != CancelFitException(), therefore the exception is never handled.
I’m not completely familiar with decorators and context managers to say with certainty that decorators are a better (easier to read and/or more efficient) route, but this is a solution I found with decorators after your suggestion, Jeremy:
Jeremy, are you accepting PRs in the course repo or is it just for the instructors?
I ask because I never contributed to an opensource project so it would be a great exercise for me, but as a professor I can totally understand if the repo is just for the instructors.
But if you are accepting PRs, do you think this change back to decorators would be an ok PR or do you prefer to just checkout earlier versions of the Learner implementation?