fix problem when learner has no more points for BalancingLearner #214
Conversation
| # Take the points from the cache | ||
| if index not in self._ask_cache: | ||
| self._ask_cache[index] = learner.ask(n=1, tell_pending=False) | ||
| points, loss_improvements = self._ask_cache[index] |
basnijholt
Sep 4, 2019
Author
Member
here one would need a double break, but because that doesn't exist I make it into a function.
|
Wow! This seems to break a lot of tests, will investigate some time later. |
|
I think that there's something I must be missing. Currently the Learner API does not allow a learner to indicate that it has no more points. I notice that the SequenceLearner actually does violate the API, which is why this "bug" appears in the first place. I would imagine that it is a complete accident that the SequenceLearner happens to work with other adaptive infrastructure (specifically the Runner). It may indeed be a good idea for learners to be able to indicate that they have no more points, but this is something that needs to be looked at carefully and should not just be implemented in an ad-hoc way. |
| self._ask_cache[index] = learner.ask(n=1, tell_pending=False) | ||
| points, loss_improvements = self._ask_cache[index] | ||
| if not points: # cannot ask for more points | ||
| return to_select |
jbweston
Oct 17, 2019
Contributor
There are a couple of things I don't understand here:
- seems this
returnshould be acontinue; just because learnericould not give any more points does not mean that no other learners can give any! - I cannot see when this branch will ever be executed.
learner.ask(1)is guaranteed to return a point. At the moment there is no way for a learner to indicate that it has "no more points". If a learner returns no points then it is in violation of the API and other stuff is liable to break
Indeed, this is #88 |
|
Seems to me that this should be closed, and we should refer to #88. @basnijholt @akhmerov upvote if you agree |
Closes #213.