fidelity / mabwiser

[IJAIT 2021] MABWiser: Contextual Multi-Armed Bandits Library
https://fidelity.github.io/mabwiser/
Apache License 2.0
213 stars 42 forks source link

Make protocols out of LearningPolicyType and NeighborhoodPolicyType? #84

Closed N-Wouda closed 1 year ago

N-Wouda commented 1 year ago

Thanks for the union types and PR for ALNS! 🎉

I noticed that currently MAB only takes learning and neighborhood policies that are already predefined as part of the LearningPolicyType or NeighborhoodPolicyType typevars. It makes sense to do this, because MAB handles each implemented policy on a case-by-case basis, using isinstance checks to decide which policy is used.

A downside to this is that users cannot easily provide their own policies without having to explicitly patch MAB (at least, if I understand the docs correctly). It might be possible to rewrite in such a way that all that a new policy needs to do is implement a few required methods. Those required methods could then be specified in an interface protocol. We do this in ALNS with the stopping and acceptance criteria, as well as the operator selection schemes: as long as users implement the required methods, their classes can be passed directly to ALNS without any modification to the library.

As I write this issue, I realize this would require significant refactoring. But it might be worthwhile because the resulting implementation would be much less coupled than it currently is.

skadio commented 1 year ago

I see the issue you are pointing at, which we also realized at some point. I love the idea and the solution too, and my reaction was "that would be quite some work.." as you hinted as well. Would make an excellent summer project for the right student ;)

N-Wouda commented 1 year ago

I would be willing to help out with that, but the issue is that I have to find the time first :-). Feel free to ping me here in a few months to see where we're at then!

skadio commented 1 year ago

Nah, don't worry about it -- this is on us