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

Maximally Interfered Retrieval plugin #1201

Merged
merged 1 commit into from Nov 24, 2022

Conversation

AlbinSou
Copy link
Collaborator

I tried to implement the ER-MIR strategy described in Online Continual Learning with Maximally Interfered Retrieval as a plugin.

The implementation for now is quite slow because of the re-instantiation of the DataLoader at every iteration. I will try to come up with a different solution but right now this is the best-working one that I have so far, so I'll leave this as a basis to improve on.

I also include a replay plugin that works similarly to the MIR one (sample two batches and add the loss coming out). It's just here so that I can compare the two methods fairly.

I thought it would be hard to implement MIR as classical avalanche replay (concatenating replay batch to current batch), since in MIR you have to perform a temporary update with the current batch before knowing what samples to use replay on. So that's why I have to compare to different replay plugin.

I also added a test script with which I try to get as close as the MIR setting described for CIFAR10, that we can use to check the results. Right now I ran only for one seed and had +4% for MIR. But I know a few details are not present in the example script, for instance I don't think they use input transformations on CIFAR10

Feel free to comment and propose improvements. I think the main idea of the method is here but few optimizations can be done still.

Copy link
Collaborator

@AntonioCarta AntonioCarta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @AlbinSou, thanks for the PR. I need to ask you for two things:

  • you have to add a test in tests/training/test_strategies. Just copy-paste the one of the other strategies and adapt it for MIR.
  • the example in examples/mir_cifar10 will go in the continual-learning-baselines repository. Can you open a PR there once this is merged?

The implementation looks good to me. Regarding the slow OCL scenario, we are investigating it with @HamedHemati. Hopefully, we will have a solution soon enough.


class ReplayLikeMIR(SupervisedPlugin):
"""
Replay plugin but working like MIR for fair comparison
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you describe the difference between the default Avalanche replay and this one here in the docstring?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @AlbinSou, for the PR!

@AntonioCarta: One question that I have is: do we want to have a general replay plugin in Avalanche that all replay-based methods can use to implement their strategies?
It, of course, makes sense and would be easier to have strategy-specific internal replay loaders (like the one in this PR) for each new plugin, but I think this plugin can also be implemented without any additional replay loaders after adding the interactive buffer mode to Avalanche ReplayPlugin. It will provide a mask for the samples in each batch so the replay loss can be calculated for buffer samples.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HamedHemati good point. A good tradeoff to avoid refactoring the implementation could be to move the "MIRLikeReplay" to the continual-learning-baselines repository as an OCL baseline, while avalanche will keep the new ReplayPlugin. What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Antonio, yes sure this MIRLikeReplay is just here for the purpose of the comparison so it has nothing to to in avalanche. I will move it to the PR in continual-baseline.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the test for now. Just to warn you, I tried to stick closer to the setting by suppressing input rotation and cropping, but this only made things worst .. Now I got 34% for MIR and 37% for replay baselines. I don't know what is wrong. I think for the most part it behaves right (after task 1 2 3 it's above normal replay but last task it drops). Maybe it's just some training artifact that I have to fix. Anyway, I guess later discussions about getting the exact same results than in the paper will happen in the continual-learning-baseline PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! as soon as you fix the pep8 errors we can merge the PR. You can check them locally with the command pycodestyle avalanche/ examples/ tests/ --exclude examples/tvdetection

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AntonioCarta Sorry, I missed the reply to my comment. My point was to implement both MIRPlugin and ReplayLikeMIR using the ReplayPlugin from Avalanche. As for MIRLikeReplay, it should be just a script to replicate results for that strategy by setting the correct arguments, and for MIRPlugin, self.replay_loader can be replaced with Avalanche's ReplayPlugin.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HamedHemati The thing that I had in mind was to implement it like that for now and later change it once the interactive buffer system is on the main branch. I don't know if this answers your question. As for ReplayLikeMIR I think it makes more sense to put it only on the continual-baseline repo since it is not very different from normal replay, I use it more to try and get close to the ER baseline the authors use in their code.

@AntonioCarta AntonioCarta mentioned this pull request Nov 17, 2022
9 tasks
@ContinualAI-bot
Copy link
Collaborator

Oh no! It seems there are some PEP8 errors! 😕
Don't worry, you can fix them! 💪
Here's a report about the errors and where you can find them:

avalanche/training/plugins/mir.py:95:81: E501 line too long (82 > 80 characters)
avalanche/training/plugins/mir.py:97:81: E501 line too long (81 > 80 characters)
avalanche/training/plugins/mir.py:173:81: E501 line too long (81 > 80 characters)
3       E501 line too long (82 > 80 characters)

@coveralls
Copy link

coveralls commented Nov 18, 2022

Pull Request Test Coverage Report for Build 3534234979

  • 69 of 72 (95.83%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 73.535%

Changes Missing Coverage Covered Lines Changed/Added Lines %
avalanche/training/supervised/strategy_wrappers.py 7 8 87.5%
avalanche/training/plugins/mir.py 54 56 96.43%
Totals Coverage Status
Change from base Build 3521832972: 0.1%
Covered Lines: 13501
Relevant Lines: 18360

💛 - Coveralls

@AntonioCarta
Copy link
Collaborator

@AlbinSou tests are still failing with this error:

ImportError: cannot import name 'ReplayLikeMIR' from 'avalanche.training.plugins.mir' (/__w/avalanche/avalanche/avalanche/training/plugins/mir.py)

probably because you removed it.

Can you also pull the master and fix the conflicts so that we can merge the PR?

@AntonioCarta
Copy link
Collaborator

Thanks @AlbinSou! Please remember to open the PR for the continual-learning-baselines repo.

@AntonioCarta AntonioCarta merged commit 8c5fd44 into ContinualAI:master Nov 24, 2022
@AlbinSou AlbinSou deleted the er_mir branch January 17, 2023 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants