Some minor fixes in batch_observation and utils #1513
Conversation
Pull Request Test Coverage Report for Build 6469661204
💛 - Coveralls |
avalanche/training/utils.py
Outdated
| if ( | ||
| training_experience.is_first_subexp | ||
| or training_experience.is_last_subexp | ||
| ): |
There was a problem hiding this comment.
can you explain this again? I'm a bit confused. With the current condition, we are going to update model and optimizer twice, at the final experience and at the new one.
There was a problem hiding this comment.
Ah, that's right, nice catch. The idea is that this util function should be able to return both sides of the boundary. Maybe I should add an option to say which side to detect ?
For instance, I had the problem recently that to implement LwF for a compatibility with online, I also need to use this function but in the after_training_exp, so I need to know the right side of the boundary, whereas when it is called in before_training_exp I need to know the left side of the boundary.
There was a problem hiding this comment.
Ok, but make it private (_at_task_boundary(before=True)). It's ok as an internal utility but I think it would be a bit confusing for external users
…ide of the boundary or another
|
if you fix the lint error I can merge this. |
Two fixes,
One is some useless code that was left in the batch_observation. This code was slowing down a bit in the case of using an online setting with access to task boundaries. I had mistakenly left this part of code when switching to using util function at_task_boundary.
Another is about some formatting and changing one feature in the at_task_boundary function. So far the function was only using is_first_subexp attribute which was leading to some problems when using it in after_training_task, it was not detecting the boundary if the experience was the last experience of the task. Now it is detecting the boundary both right before the shift and right after the shift.