updated binsearch, such that investigated quarters #14
Conversation
|
I like the idea. As we have tests for all epsilon search algorithms we should also add tests for this one in the test_epsilon_value_estimator folder |
|
Also, I think we should rename the module to make clear what exactly is adjusted compared to the binary search we have |
| class AdjustedBinarySearchEpsilonValueEstimator(BinarySearchEpsilonValueEstimator): | ||
|
|
||
|
|
||
| def get_next_epsilon(self, midpoint:int, first:int, last:int, epsilon_status_list: list[EpsilonStatus]) -> int: |
There was a problem hiding this comment.
Why is the epsilon status list needed here?
| last = midpoint - 1 | ||
| midpoint = (first + last) // 2 | ||
| else: | ||
| if len(epsilon_status_list)>3: |
There was a problem hiding this comment.
Why just in case there are more than three elements in the epsilon status list?
There was a problem hiding this comment.
if there are only 3, than its either last and first and midpoint, so the quartering might end up returning midpoint and then you get stuck in an endless loop. I will do this differently though
| midpoint = (first + last) // 2 | ||
| else: | ||
| if len(epsilon_status_list)>3: | ||
| midpoint = self.get_next_epsilon(epsilon_status_list) |
There was a problem hiding this comment.
The arguments do not match the function definition from above
|
new review required! |
| from tests.test_epsilon_value_estimator.conftest import MockVerificationModule | ||
|
|
||
|
|
||
| #TODO: adjust this for quartered |
There was a problem hiding this comment.
I think this todo should still be resolved
| class QuarteredBinarySearchEpsilonValueEstimator(BinarySearchEpsilonValueEstimator): | ||
|
|
||
|
|
||
| def get_next_epsilon(self, midpoint:int, first:int, last:int) -> int: |
There was a problem hiding this comment.
let's include spaces in the typehints
|
Did you execute the tests with the command |
Updated binary search such that if there is an error or a timeout it wont continue with the epsilon directly next to the timeout/memout epsilon, but will search further away.
We can cite Hadar Shavits work if it gets published at AAAI